[PATCH] D79590: [lld] Add a new output section ".text.unknown" for funtions with unknown hotness info especially in sampleFDO

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Writer.cpp:136
     for (StringRef v :
-         {".text.hot.", ".text.unlikely.", ".text.startup.", ".text.exit."})
+         {".text.hot.", ".text.unknown.", ".text.unlikely.", ".text.startup.", ".text.exit."})
       if (isSectionPrefix(v, s->name))
----------------
wmi wrote:
> MaskRay wrote:
> > tmsriram wrote:
> > > wmi wrote:
> > > > tmsriram wrote:
> > > > > The new prefix looks good to me.
> > > > > 
> > > > > Just curious about input sections that are named just ".text.hot" or ".text.unknown" without the traling period.  This could happen with -fno-unique-section-names I think?  In that case, this handling seems insufficient.  This is related to -keep-text-section-prefix in general. I can take a look at this.
> > > > isSectionPrefix seems considering the section name without trailing period (prefix.drop_back())?
> > > > 
> > > > ```
> > > > static bool isSectionPrefix(StringRef prefix, StringRef name) {
> > > >   return name.startswith(prefix) || name == prefix.drop_back();
> > > > }
> > > > ```
> > > Ah, thanks!
> > I know what you are discussing... (And I mentioned this problem internally previously)
> > 
> > To prevent a linker change (the GNU ld and gold approach has an small annoying behavior), I created https://reviews.llvm.org/D79600
> I am not familiar with linker so I don't know the behavior of GNU ld and gold, but lld seems recognize ".text.hot" and return ".text.hot." for it, so I don't fully understand the intention of D79600, could you explain it a little bit? Thanks.
I updated the description of D79600 a bit. Hope it is clear now... I can clarify if you still think that unclear.

In lld, I hope we only recognize `.text.exit.*` but not `.text.exit`, so that the function `exit` in a function section will not be ordered strangely.

To achieve that goal, we can rename `.text.exit` (without function sections, or with -fno-unique-section-names) to `.text.exit.`


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D79590