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

Use symver attribute not asms for symbol versioning. #92

Merged
merged 2 commits into from
Sep 12, 2020

Conversation

filbranden
Copy link
Member

Originally proposed in https://src.fedoraproject.org/rpms/numactl/pull-request/3

Comment from Jeff Law:

This package is currently opted out of LTO because it uses ASMs to implement symbol versioning. ASMs like this are fundamentally incompatible with LTO because GCC does not look inside the ASM string to find symbol references. Starting with gcc-10 there is a new symbol attribute for implementing symbol versioning. This patch changes numactl to use that symbol attribute and re-enables LTO. Note the patch includes a fallback to the old way of implementing symbol versioning via asms, so in theory it should work with clang as well as gcc-9 and earlier. I haven't fully tested the fallback path.

@filbranden filbranden force-pushed the lto3 branch 3 times, most recently from def53bc to b34d5a1 Compare September 12, 2020 03:14
filbranden and others added 2 commits September 11, 2020 20:20
Also use newer Ubuntu Bionic in tests with modern compilers. gcc-10 is not
available on the older Ubuntu version. On the other hand, revert to the older
Ubuntu Xenial for tests with gcc-4.9 and gcc-5.
@andikleen
Copy link
Contributor

Looks good to me.

@andikleen andikleen merged commit 92d4e7d into numactl:master Sep 12, 2020
This was referenced Sep 18, 2020
@filbranden
Copy link
Member Author

Small note that I followed up this one with #93 to fix hardcoded CFLAGS in some of the Makefile.am targets.

Tested that LTO builds worked on gcc 10, so all good.

Released it in numactl 2.0.14.

@filbranden filbranden deleted the lto3 branch September 18, 2020 16:39
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.

2 participants