[PATCH] D93876: Do not implicitly turn on function sections with basic block sections.

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram added inline comments.


================
Comment at: llvm/test/CodeGen/X86/basic-block-sections-blockaddress-taken.ll:10
 ; CHECK:         .text
+; CHECK:         .section .text.foo,"ax", at progbits
 ; CHECK-LABEL: foo:
----------------
dblaikie wrote:
> tmsriram wrote:
> > dblaikie wrote:
> > > Why did this test change? It looks like it ended up with unique section names + function-sections enabled, but bb-unique-section-names not enabled? But that wasn't the case prior to this patch?
> > llc did not enable function sections implicitly with basic block sections.  This patch exposed that problem.  With this patch, that gets fixed.
> Not sure I follow - the patch description says "Do not implicitly turn on function sections with basic block sections." - which seems in contradiction to the comment here about llc not enabling function sections implicitly with bb sections and this patch fixing it, and this test now having function sections when it didn't before.
TLDR; foo requires basic block sections so the entry block also gets a unique section via the newly added function.

Alright, let me explain in more detail.  Previously, llc did not turn on function sections with basic block sections.  So, even though bbsections was enabled, function foo's entry basic block did not get its own section.  That part is clear.  Just for distinction, previously clang did enable function sections implicitly with basic block sections.  So, the same test compiled with clang instead of llc would have seen a unique section for the entry basic block of foo.

Now, with basic block sections enabled, we do not create a separate section for a function that does not need basic block sections, this is with both llc and clang. However, if a function needs basic block sections, like foo in this case, we do create a unique section for its entry basic block too with the newly added function "getUniqueSectionForFunction" in TargetLoweringObjectFileImpl.cpp.

For the counter example, please test basic-block-sections-list.ll.  There, function zip does not need bb sections. So, it will not be placed in a unique function section either and the test explicitly checks for this.


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

https://reviews.llvm.org/D93876