Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable MSVC EH for Win64 #1354

Merged
merged 34 commits into from
Mar 19, 2016
Merged

Enable MSVC EH for Win64 #1354

merged 34 commits into from
Mar 19, 2016

Conversation

kinke
Copy link
Member

@kinke kinke commented Mar 12, 2016

Enable @rainers #1168 for Win64 too and add x86 AppVeyor jobs.
Requires ldc-developers/druntime#65 and a post-3.8 LLVM (Rainer's work has uncovered a few LLVM bugs whose near-instant fixes didn't make it into 3.8 final).

@kinke kinke force-pushed the msvcEH branch 2 times, most recently from 4a68f45 to ae0f049 Compare March 12, 2016 21:51
@kinke
Copy link
Member Author

kinke commented Mar 13, 2016

What AppVeyor doesn't show is that dmd-testsuite (debug+release) finally passes, for both 32bit and 64bit MSVC targets! I.e. for the very first time for LDC (on Windows in general) in case you're wondering.
Awesome work Rainer, thanks again. And thanks to @majnemer and all the other LLVM guys. 👍

@dnadlinger
Copy link
Member

Nice!

@smolt
Copy link
Member

smolt commented Mar 13, 2016

I second that 👍

Just noticed the Add You Reaction drop down. This deserves the party hat.

@rainers
Copy link
Contributor

rainers commented Mar 13, 2016

@kinke Thanks for pushing this.

And thanks to @majnemer and all the other LLVM guys

I second that, too.

@kinke
Copy link
Member Author

kinke commented Mar 13, 2016

To recap: the only (very minor) issues leading to test failures on Windows after this are:

@redstar
Copy link
Member

redstar commented Mar 13, 2016

Very well done! @kinke I think you deserve the honor to push the merge button. :-)

rainers added 19 commits March 13, 2016 17:27
… as "noreturn" by the optimizer

- do not inline functions into the cleanup funclets, it can break exception handling
- even if no var given, retrieve the caught exception to pass it to _d_eh_enter_catch
@kinke kinke force-pushed the msvcEH branch 2 times, most recently from 9c6f491 to ca1241f Compare March 16, 2016 18:19
Newest LLVM takes care of this for us.
@kinke kinke force-pushed the msvcEH branch 5 times, most recently from d67dd7d to 7ae0223 Compare March 17, 2016 01:24
@kinke
Copy link
Member Author

kinke commented Mar 17, 2016

dmd-testsuite is now enabled too for the AppVeyor jobs (it doesn't take that much longer). There are some minor path-related issues I don't have on my local system, probably due to a different set of GNU utils (I assume), e.g.:

... compilable/test9680.sh
Error: Could not create output directory 'C:C:/Program Files/Git/projects/ninja-ldc/dmd-testsuite-debug/compilable'.

Otherwise, there are only issues wrt. Dan's new runnable\ldc_cabi1.d (on my system too - I've previously used a dmd-testsuite from a couple of weeks ago). The x64 job hits LLVM assertions - apparently it doesn't like the align 16 attribute:

Wrong types for attribute: byval inalloca nest noalias nocapture nonnull readnone readonly signext sret zeroext dereferenceable(1) dereferenceable_or_null(1)
  call void (i32, ...) @cvfun(i32 3, %ldc_cabi1.F4 noalias nocapture align 16 %509), !dbg !913
Wrong types for attribute: byval inalloca nest noalias nocapture nonnull readnone readonly signext sret zeroext dereferenceable(1) dereferenceable_or_null(1)
  call void (i32, ...) @cvfun(i32 4, %ldc_cabi1.D4 noalias nocapture align 16 %536), !dbg !918
Wrong types for attribute: byval inalloca nest noalias nocapture nonnull readnone readonly signext sret zeroext dereferenceable(1) dereferenceable_or_null(1)
  call void (i32, ...) @cvfun(i32 5, %ldc_cabi1.DHFA2 noalias nocapture align 16 %552), !dbg !921
Wrong types for attribute: byval inalloca nest noalias nocapture nonnull readnone readonly signext sret zeroext dereferenceable(1) dereferenceable_or_null(1)
  call void (i32, ...) @cvfun(i32 6, %ldc_cabi1.DHFA2a noalias nocapture align 16 %568), !dbg !924
LLVM ERROR: Broken function found, compilation aborted!

The x86 job uncovers mangling issues:

ldc_cabi2.cpp.obj : error LNK2019: unresolved external symbol "signed char a" (?a@@3CA) referenced in function _cretb1
ldc_cabi2.cpp.obj : error LNK2019: unresolved external symbol "signed char b" (?b@@3CA) referenced in function _cretb2
ldc_cabi2.cpp.obj : error LNK2019: unresolved external symbol "signed char c" (?c@@3CA) referenced in function _cretf4
ldc_cabi2.cpp.obj : error LNK2019: unresolved external symbol "signed char d" (?d@@3CA) referenced in function _cretf4
ldc_cabi2.cpp.obj : error LNK2019: unresolved external symbol "unsigned int errors" (?errors@@3IA) referenced in function _cretb1

Oh and dmd-testsuite's exit code somehow isn't ignored, although I use ctest ... || echo .... That'll need to be fixed too.

@smolt
Copy link
Member

smolt commented Mar 17, 2016

The mangling error I think is a flaw in ldc_cabi2.cpp. Those vars should be extern "C".

@kinke
Copy link
Member Author

kinke commented Mar 17, 2016

Those vars should be extern "C".

That's right, I'll fix them, thanks for the hint. Edit: Why hasn't this hit us before then? Let's see how Travis reacts...
Now we hit some ldc_cabi1 failures for Win32.
The (current) Win64 issue only applies to variadic functions.

@kinke
Copy link
Member Author

kinke commented Mar 17, 2016

dmd-testsuite passes again on Win64, both on my box and for the AppVeyor x64 Debug job.

@kinke
Copy link
Member Author

kinke commented Mar 18, 2016

The Win32 ldc_cabi1 failures are also vararg related. Fix is on its way.

@kinke kinke force-pushed the msvcEH branch 2 times, most recently from 3c7f55a to 65c868a Compare March 18, 2016 19:51
As for regular args due to LDC issue ldc-developers#1356.
This fixes runnable/ldc_cabi1 and so dmd-testsuite passes again.
@kinke
Copy link
Member Author

kinke commented Mar 18, 2016

Alright, now all tests are enabled for AppVeyor, but lit test failures are ignored for x86 jobs because of align.d failing due to issue #1356.

The debug jobs are green. The release ones fail (and are allowed to fail)

This really elevates Windows MSVC to a first class target for LDC. As I mentioned earlier, it depends on a post-3.8 LLVM. @rainers even included a TLS alignment bugfix for Windows < 8.1. The recent addition of alignment attributes for IR arguments has uncovered quite a few alignment issues due to the optimizer apparently making good use of that information, so that's a good sign too.

@smolt
Copy link
Member

smolt commented Mar 18, 2016

This is awesome!

kinke added a commit that referenced this pull request Mar 19, 2016
@kinke kinke merged commit 1481dea into ldc-developers:master Mar 19, 2016
@kinke kinke deleted the msvcEH branch March 19, 2016 10:17
- dmd2\windows\bin\dmd.exe --version
# Download & extract GNU make + utils (for dmd-testsuite)
- bash --version
- ps: Start-FileDownload 'https://dl.dropboxusercontent.com/s/d0cqmxf9arbjn27/make-3.81.7z?dl=0' -FileName 'make.7z'
Copy link
Contributor

@CyberShadow CyberShadow Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, just wanted to ask, what's the original source of this file?
(Working on a test command for Digger...)

Figured it out, GnuWin32

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants