-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 clang -fwhole-program-vtables when possible #21825
base: main
Are you sure you want to change the base?
Conversation
if settings.RELOCATABLE or settings.LINKABLE or '-fPIC' in user_args: | ||
# Whether this compile command is emitting something that will only be used in | ||
# a whole-program link, that is, without dynamic linking being possible. | ||
for_whole_program_link = not settings.RELOCATABLE and not settings.LINKABLE and '-fPIC' not in user_args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about inverting this condition and the name of this variable.
e.g.
dylink_supported = settings.RELOCATABLE or settings.LINKABLE or '-fPIC' in user_args
Then I believe the '-fwhole-program-vtables' should be added only when this is not true... so it would not be in the same block as `-fvisibility=default' (which is for relocatable output).
Unfortunately this fails many tests in LTO and ThinLTO, e.g.
At least it is an error in the wasm backend and not generic code. @sbc100 any idea? Or if not, who knows |
Thats probably me yes. This looks like a basic assumption of the writer if failing, but I can't imagine why. I'll try to take a look soon. |
Emscripten is aware at compile time whether an object file will be used
in a dynamic linking context, so we are within our rights to apply that
flag in those situations, when doing LTO.
Hopefully this does not run into any LLVM LTO bugs...
This reduces 35% of
call_indirect
instructions in the Bullet benchmark(making them direct calls, some of whom end up inlined), so it may be
very useful in principle.
I added a new test here, in the first commit which is before the
optimization. The second commit shows the effect on that test: just a
few bytes of size difference, so on simple C++ programs without large
vtable hierarchies this is not expected to help much I suppose.