[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