[PATCH] D73674: Propeller: LLVM support for basic block sections (Base Patch - Part 2)

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a subscriber: kparzysz.
efriedma added a comment.

I'm afraid the way this patch is written, we're going to have to substantially rewrite it to port to other targets.  In particular, we're assigning blocks to sections in the asmprinter, which is way too late for targets that do branch relaxation as an MIR pass.

Is there some reason to emit "cold" and "exception" blocks into separate sections?  Not sure what benefit you get from separating them.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3030
+    if (MBB.isExceptionSection()) {
+      if (MF->front().isExceptionSection()) {
+        OutStreamer->SwitchSection(MF->getSection());
----------------
I'm confused what `MF->front().isExceptionSection()` is supposed to check.  The entry block can't be an exception handler, I think?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3042
+          getObjFileLowering().getNamedSectionForMachineBasicBlock(
+              MF->getFunction(), MBB, TM, ".cold"));
+    } else if (MBB.isBeginSection() && MBB.isEndSection()) {
----------------
I don't think the linker treats the ".cold" suffix specially.  Maybe we should do something different?  (There's special support in binutils ld for the prefix ".text.unlikely."; not sure how useful that would be in this context.)


================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:363
+  // This should only be done once no matter how many times it is called.
+  if (this->BBSectionsSorted || !this->getBBSections())
+    return false;
----------------
Can this actually get called more than once?

Why do we care if the blocks are sorted?  Does it help optimize the code somehow?


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

https://reviews.llvm.org/D73674