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

[Wei Mi via Phabricator via llvm-commits]

wmi marked an inline comment as done.
wmi 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))
----------------
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.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D79590