Page MenuHomePhabricator

[InstrProf] Mark __llvm_profile_runtime hidden to match libclang_rt.profile definition
ClosedPublic

Authored by justincady on Jun 29 2022, 11:25 AM.

Details

Summary

Mark the symbol hidden to match INSTR_PROF_PROFILE_RUNTIME_VAR in compiler-rt.

Fixes second issue discussed at https://discourse.llvm.org/t/63090

Diff Detail

Event Timeline

justincady created this revision.Jun 29 2022, 11:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2022, 11:25 AM
justincady requested review of this revision.Jun 29 2022, 11:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2022, 11:25 AM
MaskRay accepted this revision.Jun 29 2022, 4:03 PM

This change does not mark the symbol hidden on Windows to match libclang_rt.profile's default visibility for Windows targets.

Just drop the special case.

This change looks good to me because of COMPILER_RT_VISIBILITY extern int INSTR_PROF_PROFILE_RUNTIME_VAR;

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
1247

Remove Windows special case. The visibility is a no-op on Windows.

This revision is now accepted and ready to land.Jun 29 2022, 4:03 PM

Remove special case for Windows based on MaskRay's comment.

justincady added inline comments.Jun 30 2022, 7:21 AM
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
1247

Done, thank you. I was trying to match COMPILER_RT_VISIBILITY not setting visibility for Windows, but did not realize it isn't set because it's a no-op.

justincady edited the summary of this revision. (Show Details)Jun 30 2022, 7:22 AM
justincady marked an inline comment as done.Mon, Jul 11, 10:49 AM

@MaskRay Does this change require any further review? If it is good to go, would you please land it for me? I don't have commit access.

Thanks!

This revision was landed with ongoing or failed builds.Mon, Jul 11, 11:29 AM
This revision was automatically updated to reflect the committed changes.