[compiler-rt] [llvm] [RFC][Transforms][IPO] Add func suffix in ArgumentPromotion and DeadArgumentElimination (PR #109899)
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 7 19:25:12 PDT 2024
teresajohnson wrote:
> @teresajohnson @arsenm I tried sampling based profiling with one of bpf applications and it looks like the added suffixes are not affecting sampling based profiling. See the details in the above. I also marked the patch as RFC as you suggested.
Regarding that analysis, I just want to clarify: you are showing that the profiled binary has the new suffixes in its symbol table, but that the dwarf data for the same binary does not have the new suffixes, and that llvm-profgen will construct the profile from the dwarf so not contain the suffixes? I am not very familiar with llvm-profgen so defer to @huangjd. It would be good to confirm with a round trip through the feedback path that things work as expected.
> For func suffixes (or more than one suffixes), gcc already has precedences. The below are some examples when build clang with gcc:
>
> ```
> $ llvm-readelf -s clang | grep isra | grep constprop.
> ...
> 135408: 00000000061d7140 1529 FUNC LOCAL DEFAULT 14 _ZN4llvm4yaml7yamlizeISt6vectorIN12_GLOBAL__N_15ParamESaIS4_EENS0_12EmptyContextEEENSt9enable_ifIXsrNS0_18has_SequenceTraitsIT_EE5valueEvE4typeERNS0_2IOERSA_bRT0_.constprop.0.isra.0
> 135415: 00000000061de7b0 6558 FUNC LOCAL DEFAULT 14 _ZN4llvm4yaml7yamlizeISt6vectorIN12_GLOBAL__N_15ClassESaIS4_EENS0_12EmptyContextEEENSt9enable_ifIXsrNS0_18has_SequenceTraitsIT_EE5valueEvE4typeERNS0_2IOERSA_bRT0_.constprop.0.isra.0
> 135416: 00000000061e0150 1755 FUNC LOCAL DEFAULT 14 _ZN4llvm4yaml7yamlizeISt6vectorIN12_GLOBAL__N_18FunctionESaIS4_EENS0_12EmptyContextEEENSt9enable_ifIXsrNS0_18has_SequenceTraitsIT_EE5valueEvE4typeERNS0_2IOERSA_bRT0_.constprop.0.isra.0
> 135417: 00000000061e0830 5534 FUNC LOCAL DEFAULT 14 _ZN4llvm4yaml7yamlizeISt6vectorIN12_GLOBAL__N_13TagESaIS4_EENS0_12EmptyContextEEENSt9enable_ifIXsrNS0_18has_SequenceTraitsIT_EE5valueEvE4typeERNS0_2IOERSA_bRT0_.constprop.0.isra.0
> 135419: 00000000061e3ab0 1502 FUNC LOCAL DEFAULT 14 _ZN4llvm4yaml7yamlizeISt6vectorIN12_GLOBAL__N_19VersionedESaIS4_EENS0_12EmptyContextEEENSt9enable_ifIXsrNS0_18has_SequenceTraitsIT_EE5valueEvE4typeERNS0_2IOERSA_bRT0_.constprop.0.isra.0
> ```
>
> And there are even some cases having three suffixes:
>
> ```
> $ llvm-readelf -s clang | grep isra | grep constprop | grep part
> 5663: 000000000147dd70 1041 FUNC LOCAL DEFAULT 14 _ZN4llvm10GCNTTIImpl18getVectorInstrCostEjPNS_4TypeENS_19TargetTransformInfo14TargetCostKindEjPNS_5ValueES6_.part.0.constprop.1.isra.0
> 5664: 000000000147e190 1041 FUNC LOCAL DEFAULT 14 _ZN4llvm10GCNTTIImpl18getVectorInstrCostEjPNS_4TypeENS_19TargetTransformInfo14TargetCostKindEjPNS_5ValueES6_.part.0.constprop.0.isra.0
> ```
>
> So if new suffixes in clang won't affect functionality, then it should be okay for clang as well to allow multiple suffixes.
>
> Please let me know what you think.
Except we don't tend to feed back profiles collected from gcc built binaries to clang for SamplePGO, etc, so we need to ensure it will still work in clang.
https://github.com/llvm/llvm-project/pull/109899
More information about the llvm-commits
mailing list