[PATCH] D146853: [Pseudo Probe] Placing .pseudoprobe section in the same comdat group with .text

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 16 13:42:57 PDT 2023


hoy added a comment.

In D146853#4429307 <https://reviews.llvm.org/D146853#4429307>, @MaskRay wrote:

> In D146853#4429306 <https://reviews.llvm.org/D146853#4429306>, @hoy wrote:
>
>> In D146853#4429265 <https://reviews.llvm.org/D146853#4429265>, @MaskRay wrote:
>>
>>> In D146853#4428709 <https://reviews.llvm.org/D146853#4428709>, @hoy wrote:
>>>
>>>> In D146853#4428673 <https://reviews.llvm.org/D146853#4428673>, @MaskRay wrote:
>>>>
>>>>> Thank you for mentioning lld/ELF, otherwise I may not notice this patch:) The part is not so related to the main functionality and I usually have a strong opinion on lld/test/ELF testing...
>>>>
>>>> A test on lld side sounds good. I'll add it.
>>>>
>>>>> Actually, for LLVM metadata sections, I think the convention is not to have a suffix like `.text.foo`. You may want to use `.pseudo_probe` without the suffix (`MCSectionELF::UniqueID`).
>>>>
>>>> D152546 <https://reviews.llvm.org/D152546> is one implementation that treats `.pseudo_probe` as an independent section of `.text`, but after iterations we think they should be combined when it comes to deduplication and gc-sections, so I'm falling back to this patch.
>>>
>>> My idea is that we should not have `.pseudo_probe.xxx` (waste `.strtab` size). Just have a number of `.pseudo_probe` (either differentiated by a comdat key or a `UniqueID`). This scheme matches many other metadata sections from all sorts of instrumentations.
>>
>> Is it required that sections of a comdat group should have a suffix same to the group name?  E.g, `.rela.text.foo`, is it fine to remove the function name from it?
>
> There is no such requirement. LLVM instrumentation generally omits the suffix for metadata sections to save `.strtab` size (related to relocatable object file sizes).

I see. BTW, why is ``UniqueID` required? I'm seeing even with `MCSection::NonUniqueID` the sections are still separate, something like


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146853/new/

https://reviews.llvm.org/D146853



More information about the llvm-commits mailing list