Improve user experience with ldc2, incl. cross-compilation #1755

kinke · 2019-08-17T14:27:19Z

No description provided.

Don't perform the does-default-target-triple-contain-msvc check on non-Windows hosts (which would need a hacked LLVM to return true on other hosts).

dlang-bot · 2019-08-17T14:27:20Z

Thanks for your pull request and interest in making D better, @kinke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

kinke · 2019-08-17T20:08:06Z

@skoppe: Could you give this a try and see if it'd be a viable solution for #1749?

kinke · 2019-08-17T20:35:17Z

Other related issues: #809, #1431, #1523, #1701.

skoppe · 2019-08-17T20:37:16Z

source/dub/platform.d

@@ -42,6 +42,7 @@ enum string platformCheck = q{
 	version(Android) ret ~= "android";
 	version(Cygwin) ret ~= "cygwin";
 	version(MinGW) ret ~= "mingw";
+	version(WebAssembly) ret ~= "wasm";


This is good for completeness sake, but it won't do much, since it only ever runs when compiling dub itself to WebAssembly.

Nope, this is also mixed into a platform probe file. The compiler compiles that one, using dub's --arch as LDC -mtriple, so that the BuildPlatform (parsed from compiler output) will feature a wasm platform. It's what makes dub 'target-aware'.

skoppe · 2019-08-17T20:39:28Z

@skoppe: Could you give this a try and see if it'd be a viable solution for #1749?

So I just compile with dub -arch=wasm32-unknown-unknown-wasm?

kinke · 2019-08-17T20:55:58Z

Yep, and remove the -mtriple option in the dub config files.

skoppe · 2019-08-17T21:25:50Z

I can see that dub passes the triple to ldc, but it also looks the dflags in the dub.sdl are stripped, including the betterC I need to pass compilation.

This is basically dlang#1548 with an augmented set of compiler flags being included in the linking command lines.

By treating all 'architectures' containing a `-` as `-mtriple` values. E.g., `dub --arch=wasm32-unknown-unknown-wasm` to cross-compile to WebAssembly. This dub switch is already used when probing the compiler, so the build platform detection works as expected. Also add `wasm` as detected platform and use the .wasm executable file extension (for ldc2). This fixes dlang#1749 when moving the `-mtriple` option from the dub config file to the dub command line.

kinke · 2019-08-18T09:39:21Z

I don't see how previous dflags wouldn't be applied anymore (after the amended fix wrt. a -betterC dflag making it into link cmdlines too when linking separately). dub configs + resulting cmdlines would be helpful. [My attempts at building a spasm dummy project fail due to the optional package dependency not compiling for wasm (missing wchar_t alias in druntime - only defined for version (Windows) and version (Posix) - etc).]

How is that supposed to work anyway? I mean, each (transitive) dependency of spasm would ideally be compiled with -betterC -fvisibility=hidden (-mtriple already is via --arch). Is there a way to enforce some base flags for the whole package tree?

With --arch, the build (target) platform might now diverge from the host platform. The compiler's default triple doesn't matter when cross-compiling, so use the <name>.lib MSVC format when targeting Windows but not MinGW. This fixes one aspect of dlang#1599.

skoppe · 2019-08-18T11:16:51Z

How is that supposed to work anyway? I mean, each (transitive) dependency of spasm would ideally be compiled with -betterC -fvisibility=hidden (-mtriple already is via --arch). Is there a way to enforce some base flags for the whole package tree?

No there is not. That is a short coming of dub. Properties are only propagated upwards.

With regards to optional, I made a pull request for it to be betterC compatible (and merged). That has worked in the past. But with this pr I get similar errors to you.

I looked at the compiler invocations and with the current dub, the optional package isn't actually build for wasm, no triple, no betterC.

With this pr the wasm triple gets propagates downwards and causes issues. Well, rightfully so. I made a pr for the optional package to fix its configuration see pull#38

The reason it works is that there are only templates in the package.

kinke · 2019-08-18T12:30:17Z

That is a short coming of dub. Properties are only propagated upwards.

Oh wow, a serious shortcoming indeed. One could apparently work something out with the DFLAGS env variable, but I would have expected dub support for some global --dflags.

But with this pr I get similar errors to you.

This is off-topic, but DMD only supports either version (Windows) or version (Posix), while LDC predefines none of those 2 for Android, WebAssembly and unknown OS. Predefining Posix (currently disallowed in cmdline) for WebAssembly would get around these compile errors.

skoppe · 2019-08-18T12:50:36Z

This is off-topic, but DMD only supports either version (Windows) or version (Posix), while LDC predefines none of those 2 for Android, WebAssembly and unknown OS. Predefining Posix (currently disallowed in cmdline) for WebAssembly would get around these compile errors.

also off-topic:

I have a wip PR in druntime where I define all those things for WebAssembly. Actually I took the WASI headers so it isnt strictly wasm. My reasoning is that is if you are using libc/druntime in wasm, you are actually either targeting WASI or emscripten. And if you don't, you simply dont call them and they will never end up in the binary.

Regardless, I don't think setting the Posix version for WebAssembly is the right thing to do. I remember trying it briefly just to get things compiling without betterC, but I abandoned that in favor of wasi.

skoppe · 2019-08-18T12:54:04Z

@skoppe: Could you give this a try and see if it'd be a viable solution for #1749?

So I just compile with dub -arch=wasm32-unknown-unknown-wasm?

If I fix the above mentioned issues with optional (and mir-core as well), I can confirm that dub now sets the file extensions correctly.

Then I get this error:

lld: error: unknown argument: --no-as-needed

with

LDC - the LLVM D compiler (1.17.0-beta1):
  based on DMD v2.087.1 and LLVM 8.0.1
  built with LDC - the LLVM D compiler (1.17.0-beta1)

kinke · 2019-08-18T12:55:20Z

I have a wip PR in druntime where I define all those things for WebAssembly.

Ah nice, yeah that's a proper solution. I assume it cannot be tested for regressions though?

kinke · 2019-08-18T13:00:42Z

Then I get this error

Thx, should be fixed now.

skoppe · 2019-08-18T13:15:45Z

Oh wow, a serious shortcoming indeed. One could apparently work something out with the DFLAGS env variable, but I would have expected dub support for some global --dflags.

I have hit that issue before with the dip1000/dip25 compiler flag. The issue was that a dependency had a template which I used in a project where dip1000/dip25 was active. It caused mangling issues because the template called a regular function in the dependency, which had different function attributes inferred on it (IIRC it was the return attribute). The workaround was to use the sourceLibrary targetType.

The obvious solution would be to propagate all dips (or other future compiler feature flags) to all dependencies. But I don't think that will work, as it is perfectly valid for dependencies to not use various compiler feature flags. Although with something as determent as an arch, it seems more than reasonable to propagate it.

skoppe · 2019-08-18T13:21:49Z

Thx, should be fixed now.

Works!

kinke · 2019-08-18T13:22:24Z

Although with something as determent as an arch, it seems more than reasonable to propagate it.

Yep, I think there's a class of flags which make perfect sense as base flags, incl. -betterC, -linker, -L, -Xcc, -fvisibility, -flto (yes, that should suffice for LTO across all packages) etc. I'd leave it up to the user to choose sane ones.

…latform runtime checks) Also fixes dlang#1249. For LDC as implicit cross compiler, the file extensions for libraries and object files etc. don't depend on the compiler, but solely on the target. Targets selected with `--arch` are correctly probed by dub, so that the parsed `BuildPlatform` represents the target platform.

andre2007 · 2019-08-21T20:09:01Z

@kinke the dub argument for just printing what would be done is --annotate.

Geod24

Not a big deal if the test isn't here IMO, especially given it's currently pretty hard to test this kind of things.

Geod24 · 2019-08-25T09:15:38Z

Ping @wilzbach @thewilsonator , it would be great to have this in the next release.

MartinNowak · 2019-08-28T22:17:03Z

A new feature right before the release candidate?
This change could use a test case and a changelog entry for the cross-compilation.

Geod24 · 2019-08-29T00:57:11Z

A new feature right before the release candidate?

A lot of bug fixes, that only affect LDC, and were PRed by an LDC maintainer.
Basic things like coverage and profiling didn't even work with LDC and didn't give any feedback about not working.

This includes dlang/dub#1755 which improves LDC support in general and enables specifying arbitrary LLVM target triples as dub `--arch` option.

Changelog follow-up to #1755 merged-on-behalf-of: Nicholas Wilson <[email protected]>

jondegenhardt · 2019-10-30T19:12:01Z

Yep, I think there's a class of flags which make perfect sense as base flags, incl. -betterC, -linker, -L, -Xcc, -fvisibility, -flto (yes, that should suffice for LTO across all packages) etc. I'd leave it up to the user to choose sane ones.

@kinke - Building with LTO such that it included dependent packages would be very useful. Is this what you have in mind?

kinke · 2019-10-30T19:25:46Z

Yes; should be doable today via DFLAGS='-flto=thin' dub ....

skoppe · 2019-10-30T19:36:21Z

Yep, I think there's a class of flags which make perfect sense as base flags, incl. -betterC, -linker, -L, -Xcc, -fvisibility, -flto (yes, that should suffice for LTO across all packages) etc. I'd leave it up to the user to choose sane ones.

I would also include -conf to that list. I am currently porting druntime to wasm and my setup includes having a base package that defines a custom ldc.conf. It works well for dependent packages (they inherit the conf), but falls short when using dependencies.

jondegenhardt · 2019-10-31T03:58:23Z

Yes; should be doable today via DFLAGS='-flto=thin' dub ....

Hm... I thought there was more to it. I'm partly thinking of having dependent packages also pull in phobos/druntime via -defaultlib=phobos2-ldc-lto,druntime-ldc-lto, but maybe that'd happen automatically as well.

I'll look into this further and see if it basically takes care of itself or there are other details to be worked through.

kinke added 2 commits August 17, 2019 15:51

Support a few more ldc2 command-line options

c12b3ba

Restrict extra ldc2 invocation for static libs to Windows hosts

4cc1733

Don't perform the does-default-target-triple-contain-msvc check on non-Windows hosts (which would need a hacked LLVM to return true on other hosts).

kinke force-pushed the ldc branch from adbc7fd to c8fe948 Compare August 17, 2019 14:31

thewilsonator approved these changes Aug 17, 2019

View reviewed changes

kinke force-pushed the ldc branch from 1760711 to 7e1b58d Compare August 17, 2019 19:49

skoppe reviewed Aug 17, 2019

View reviewed changes

kinke added 2 commits August 18, 2019 00:24

Support separate linking with ldc2

c5dac5b

This is basically dlang#1548 with an augmented set of compiler flags being included in the linking command lines.

kinke force-pushed the ldc branch from 3d9abc4 to 4b96b54 Compare August 17, 2019 22:27

kinke force-pushed the ldc branch from 4b96b54 to b4e14d5 Compare August 18, 2019 09:44

ldc2: Only use -L--no-as-needed for Linux *targets*

d55e9b7

andre2007 mentioned this pull request Aug 19, 2019

Linker file ending detection should be compiler dependent #1249

Closed

Geod24 approved these changes Aug 22, 2019

View reviewed changes

This was referenced Aug 22, 2019

Support targetType 'wasm' #1541

Closed

Implement separate linking for LDC #1548

Closed

thewilsonator approved these changes Aug 25, 2019

View reviewed changes

thewilsonator added the auto-merge label Aug 25, 2019

dlang-bot merged commit 8ffc09e into dlang:master Aug 25, 2019

kinke deleted the ldc branch August 25, 2019 10:49

Geod24 mentioned this pull request Aug 26, 2019

Merge remote-tracking branch 'upstream/stable' into merge_stable #1760

Merged

kinke mentioned this pull request Sep 7, 2019

Upgrade bundled dub to v1.17.0+ (with cross-compilation support) ldc-developers/ldc#3157

Merged

kinke added a commit to kinke/dub that referenced this pull request Sep 7, 2019

Changelog follow-up to dlang#1755

1c26d00

dlang-bot added a commit that referenced this pull request Sep 7, 2019

Merge pull request #1767 from kinke/patch-1

117adc5

Changelog follow-up to #1755 merged-on-behalf-of: Nicholas Wilson <[email protected]>

andre2007 mentioned this pull request Dec 6, 2019

Define support for advanced target architecture/CPU specifications #228

Closed

valpackett mentioned this pull request Jan 26, 2020

ldc: support --arch=aarch64 and powerpc64 #1868

Merged

kinke mentioned this pull request Aug 1, 2020

Unrecognized file extension lib skoppe/spasm#15

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve user experience with ldc2, incl. cross-compilation #1755

Improve user experience with ldc2, incl. cross-compilation #1755

kinke commented Aug 17, 2019

dlang-bot commented Aug 17, 2019

kinke commented Aug 17, 2019

kinke commented Aug 17, 2019

skoppe Aug 17, 2019

kinke Aug 17, 2019 •

edited

Loading

skoppe commented Aug 17, 2019

kinke commented Aug 17, 2019

skoppe commented Aug 17, 2019

kinke commented Aug 18, 2019 •

edited

Loading

skoppe commented Aug 18, 2019

kinke commented Aug 18, 2019

skoppe commented Aug 18, 2019

skoppe commented Aug 18, 2019

kinke commented Aug 18, 2019

kinke commented Aug 18, 2019

skoppe commented Aug 18, 2019

skoppe commented Aug 18, 2019

kinke commented Aug 18, 2019

andre2007 commented Aug 21, 2019

Geod24 left a comment

Geod24 commented Aug 25, 2019

MartinNowak commented Aug 28, 2019

Geod24 commented Aug 29, 2019

jondegenhardt commented Oct 30, 2019

kinke commented Oct 30, 2019

skoppe commented Oct 30, 2019

jondegenhardt commented Oct 31, 2019

Improve user experience with ldc2, incl. cross-compilation #1755

Improve user experience with ldc2, incl. cross-compilation #1755

Conversation

kinke commented Aug 17, 2019

dlang-bot commented Aug 17, 2019

kinke commented Aug 17, 2019

kinke commented Aug 17, 2019

skoppe Aug 17, 2019

Choose a reason for hiding this comment

kinke Aug 17, 2019 • edited Loading

Choose a reason for hiding this comment

skoppe commented Aug 17, 2019

kinke commented Aug 17, 2019

skoppe commented Aug 17, 2019

kinke commented Aug 18, 2019 • edited Loading

skoppe commented Aug 18, 2019

kinke commented Aug 18, 2019

skoppe commented Aug 18, 2019

skoppe commented Aug 18, 2019

kinke commented Aug 18, 2019

kinke commented Aug 18, 2019

skoppe commented Aug 18, 2019

skoppe commented Aug 18, 2019

kinke commented Aug 18, 2019

andre2007 commented Aug 21, 2019

Geod24 left a comment

Choose a reason for hiding this comment

Geod24 commented Aug 25, 2019

MartinNowak commented Aug 28, 2019

Geod24 commented Aug 29, 2019

jondegenhardt commented Oct 30, 2019

kinke commented Oct 30, 2019

skoppe commented Oct 30, 2019

jondegenhardt commented Oct 31, 2019

kinke Aug 17, 2019 •

edited

Loading

kinke commented Aug 18, 2019 •

edited

Loading