ldc-developers / ldc2.snap

Snap package definition for LDC, the LLVM-based D compiler
11 stars 4 forks source link

Replace llvm-dev build-package with fresh build of LLVM 3.9.1 #10

Closed WebDrake closed 7 years ago

WebDrake commented 7 years ago

This ensures the snap will use the latest stable LLVM release supported by LDC. It also removes one more assumption about what libraries are available on the system on which the snap is built.

@klickverbot @JohanEngelen does this look sane for you in terms of the build setup of LLVM itself?

JohanEngelen commented 7 years ago

does this look sane for you in terms of the build setup of LLVM itself?

Yes. For LTO, you need to add -DLLVM_BINUTILS_INCDIR=/path/to/binutils/include to the LLVM CMake call. See http://llvm.org/docs/GoldPlugin.html#lto-how-to-build

Edit: "for LTO" means: to build the linker plugin that is needed for LDC LTO.

WebDrake commented 7 years ago

Great, thanks, I'll add that. Will it work also with the bootstrap compiler or should that option only be used with v1.1.0+...?

JohanEngelen commented 7 years ago

the option is for building LLVM, so it generates the plugin. Afterwards, this plugin needs to be installed, you can set -DLDC_INSTALL_LTOPLUGIN=ON to do that. (but indeed, LDC 1.1.0+ can do LTO)

WebDrake commented 7 years ago

I've rebased on master and added an extra patch to add LTO support. Are there any particular good test-cases for this feature ... ?

WebDrake commented 7 years ago

As far as I can tell the -flto option "works" inasmuch as the compiler accepts it without complaint and it makes a difference to the size of the generated executable. Beyond that, not obvious ;-)

WebDrake commented 7 years ago

@JohanEngelen just to check, are there any other changes that would be required in order for this snap package to be on par with the other official LDC packages? Or does this now cover all essential requirements?

JohanEngelen commented 7 years ago

If an executable is generated with -flto, then that is enough confirmation of LTO working (the linker would not be able to understand .o files generated with -flto. And so if that works, that puts the snap package ahead of our release packages on Github for Linux. :-)

WebDrake commented 7 years ago

Hah, nice! I'll get an updated snap package out ASAP, then ;-)

WebDrake commented 7 years ago

Just for the record, this is what I see at the linker stage if I invoke the snap-packaged ldc2 using the -flto=full option:

/usr/bin/gcc test.o -o test -Xlinker -plugin -Xlinker /snap/ldc2/x1/lib/LLVMgold-ldc.so -Xlinker -plugin-opt=O3 -L/snap/ldc2/x1/bin/../lib -lphobos2-ldc -ldruntime-ldc -Wl,--gc-sections -lrt -ldl -lpthread -lm -m64

Looks correct ... ?

WebDrake commented 7 years ago

I would guess the main question here is whether the LLVMgold-ldc.so plugin is compatible in practice with systems other than Ubuntu 16.04, depending on the compiler toolchain in use. No doubt we'll find out when the latest package is released... ;-)

JohanEngelen commented 7 years ago

linker line looks correct, yep.

The plugin is compatible with the system it is built on (the plugin-api.h file) and only works when the linker supports plugins (ld.gold definitely works, ld.bfd may work too I'm not sure).

WebDrake commented 7 years ago

The plugin is compatible with the system it is built on (the plugin-api.h file) and only works when the linker supports plugins (ld.gold definitely works, ld.bfd may work too I'm not sure).

My major concern here is ABI compatibility. The header alone should be OK so long as it's reasonable to assume the plugin API doesn't change between 2.x releases of binutils (which seems reasonable, but might be catastrophically wrong...).

I'll get to try it out tomorrow on an Ubuntu 14.04 system (from which I think 16.04 has an ABI break), so I guess I'll find out then, if no one else lets me know sooner ;-)

If it does prove problematic then I'm not sure what to do with it. It might be possible to "rescue" by including a full GCC and gold linker in the snap, but TBH I'd rather drop the LTO feature for now if that turned out to be necessary.

Updated version published in the --edge channel of the snap store just a little while ago, BTW, if you want to try it out yourself :-)

JohanEngelen commented 7 years ago

I thought the snap package is rebuilt from source for every system? (GCC cannot do anything with LLVM/LDC LTO by the way)

WebDrake commented 7 years ago

I thought the snap package is rebuilt from source for every system?

Depends what you mean by "system". They are meant to be "universal" binary packages inasmuch as they are not distro-specific, but they are built separately for specific architectures. Basically, I build the binary package and upload it (currently only for amd64), and it should work on any system supporting the snapd daemon.

The price for that is that certain things inside the snap need to be reliably self-contained. Relying on an external linker could be a breach of that, depending on the exact assumptions made by the plugin.

(GCC cannot do anything with LLVM/LDC LTO by the way)

Interesting, because it's GCC that seems to have been invoked for linking by LDC when I used the -flto=full option, and it seemed to take the various -Xlinker arguments without complaint. There was a noticeable difference to the size of the generated binaries, which ran without problem.

JohanEngelen commented 7 years ago

Yes, we use the system compiler to invoke the system linker. -Xlinker passes the following argument to the linker. Definitely if things link, then LTO is working. The "object files" that LDC outputs with -flto are not object files at all; they are LLVM IR files in disguise.

WebDrake commented 7 years ago

Great, then as I said, it probably comes down to whether there are any ABI issues with the plugin on systems other than Ubuntu 16.04.

BTW, on a slightly related note (more about multi-arch support, perhaps): I'm going to make looking into CI support the next major priority. That'll probably be easier as of next week, when snapcraft 2.27 should be released with some extra support in this respect for 'classic' snaps.