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

[BOLT] Emit synthetic FILE symbol for local cold fragments of global symbols #89794

Merged
merged 5 commits into from
Apr 25, 2024

Conversation

aaupov
Copy link
Contributor

@aaupov aaupov commented Apr 23, 2024

Follow-up to #89648.

Emit a FILE symbol to distinguish BOLT-created local symbols for cold
fragments of global split functions, to prevent polluting the namespace
of the last file symbol.

This eliminates the possibility of emitting the fragment under the file
that already contains the parent and fragment with the same name, which
can break cold fragment parent lookup in registerFragment.

This issue is more probable with LTO binaries where many symbols are
emitted under ld-temp.o.

Test Plan: Updated cdsplit-symbol-names.s

Follow-up to llvm#89648.
Emit a FILE symbol to distinguish BOLT-created local symbols for split
functions, to prevent polluting the namespace of the last file symbol.

Test Plan: Updated cdsplit-symbol-names.s
@aaupov aaupov marked this pull request as ready for review April 24, 2024 23:22
@aaupov aaupov changed the title [BOLT] Emit bolt-synthetic FILE symbol [BOLT] Emit synthetic FILE symbol for local cold fragments of global symbols Apr 24, 2024
Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate a little more on why we need this?

bolt/lib/Rewrite/RewriteInstance.cpp Outdated Show resolved Hide resolved
Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@aaupov aaupov merged commit 090c92e into llvm:main Apr 25, 2024
3 of 4 checks passed
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

3 participants