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

Avoid naked DMD-style asm functions & enable LTO on Win64 #2774

Merged
merged 9 commits into from Aug 20, 2018

Conversation

kinke
Copy link
Member

@kinke kinke commented Jul 12, 2018

No description provided.

@kinke
Copy link
Member Author

kinke commented Jul 12, 2018

Just checking CI status for now. This is more work than I expected; ldc.eh_msvc and std.internal.digest.sha_SSSE3 are still missing (and non-trivial). There's some more DMD-naked, but for non-Windows only.

@kinke kinke force-pushed the noNakedDMD branch 4 times, most recently from e4b5cbd to 1ea429a Compare July 14, 2018 00:22
@kinke
Copy link
Member Author

kinke commented Jul 14, 2018

The LTO default libs can now be built for Win64. Building and running a Phobos-hello-world with external LLD linker working via ldc2 -flto=thin -defaultlib=phobos2-ldc-lto,druntime-ldc-lto -linker=lld-link (with full LTO and/or -O too). :)

@kinke kinke force-pushed the noNakedDMD branch 3 times, most recently from 5f9ca4c to 4ff655b Compare July 14, 2018 02:38
@kinke kinke changed the title WIP: Avoid naked DMD-style asm functions WIP: Avoid naked DMD-style asm functions / enable LTO on Win64 Jul 14, 2018
@dnadlinger
Copy link
Member

I'm somewhat wary of the extra maintenance overhead, but I guess it's acceptable, as LTO is an important use case.

Idea: What about a global version that can be specified on the command line to use the upstream asm, so we can easily test whether we can remove the workaround in the future?

LLVM might be fixed at some point, or we might reimplement naked asm to lower to a naked function rather than module-level asm.

@kinke
Copy link
Member Author

kinke commented Jul 15, 2018

I'm somewhat wary of the extra maintenance overhead, but I guess it's acceptable, as LTO is an important use case.

Yeah, not too happy about the duplicated code in core.thread either. Also not sure if Rainer likes the AT&T syntax in ldc.eh_msvc. ;)

Idea: What about a global version that can be specified on the command line to use the upstream asm, so we can easily test whether we can remove the workaround in the future?

Hmm, we could also just make an existing LTO lit-test with naked DMD asm xfail on Win64. Enabling the LTO lit-tests would be the next step anyway (+ integration test for AppVeyor, maybe even bundling lld-link.exe in the release package).

we might reimplement naked asm to lower to a naked function rather than module-level asm.

I was thinking about that too, may be a good idea in general (inline-able etc.).

@dnadlinger
Copy link
Member

I was thinking about that too, may be a good idea in general (inline-able etc.).

This is somewhat of a tangent, but I don't think we should ever inline naked asm – the whole point of it is to be able to rely on the platform ABI defaults w.r.t. stack frame setup, arguments in registers, etc. If you don't make use of this, there is no point in using naked in the first place. (IIRC there are a couple of places in Phobos where it is used as a microoptimization to elide the DMD-generated prolouge, but that's misusing one feature to cover for defects in another…)

@kinke kinke force-pushed the noNakedDMD branch 2 times, most recently from 19ac80a to 083b7ca Compare July 28, 2018 18:06
@kinke kinke added this to the 1.12.0 milestone Jul 28, 2018
@kinke kinke force-pushed the noNakedDMD branch 3 times, most recently from 3f585fa to 6e171c6 Compare July 28, 2018 22:20
@kinke kinke changed the title WIP: Avoid naked DMD-style asm functions / enable LTO on Win64 Avoid naked DMD-style asm functions & enable LTO on Win64 Jul 29, 2018
@kinke
Copy link
Member Author

kinke commented Jul 29, 2018

Alright, LTO lit tests enabled for Windows, integration test added with LTO default libs and lld-link.exe bundled with x64 and multilib packages. Parameter-less impl-function workaround not needed anymore with updated @naked semantics.

@dnadlinger
Copy link
Member

Nice!

@kinke
Copy link
Member Author

kinke commented Jul 29, 2018

[This fixes #628 and #2755.]

@kinke kinke closed this Jul 29, 2018
@kinke kinke reopened this Jul 29, 2018
There are some remaining functions, but not for Windows, where they
block LTO.
Just a command-line fix for the rename command and enabling
RT_ARCHIVE_WITH_LDC by default on Windows too.
On Win32, there's still naked DMD-style asm, e.g., in
std.internal.digest.sha_SSSE3.
* Imply `-flto=thin` when compiling object & bitcode files.
  D_FLAGS don't need to be touched anymore.
* Don't emit bitcode files when compiling unittests (and shared druntime
  objects etc.).

The latter is necessary on Win64, as there's (non-trivial) naked DMD-
style asm in a core.thread unittest.
If lld-link.exe is found in PATH.

Make tests/linking/thinlto_asm_x86.d XFAIL on Windows, so that we are
notified when/if that LLVM bug is fixed.
And test portability too.
@kinke kinke merged commit feca5bc into ldc-developers:master Aug 20, 2018
@kinke kinke deleted the noNakedDMD branch August 20, 2018 09:23
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.

None yet

2 participants