[PATCH] D76954: LLVM support for BB-cluster sections

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1217
+          emitELFSizeDirective(CurrentSectionBeginSym);
+        MBB.setEndMCSymbol(CurrentBBEnd);
+      }
----------------
It looks like getEndMCSymbol is unused?  (And therefore setEndMCSymbol/EntrySectionEndMBB are dead code.)


================
Comment at: llvm/lib/CodeGen/MIRParser/MIRParser.cpp:51
+/// and the SectionID's are assigned to MBBs.
+static void assignBeginEndSections(MachineFunction &MF) {
+  MF.front().setIsBeginSection();
----------------
rahmanl wrote:
> efriedma wrote:
> > Why are there two copies of assignBeginEndSections ?
> This function is defined to set IsBeginSection and IsEndSection which are derived from SectionID fields and the MBB order of the function. So we just need to iterate over all MBBs once and assign the fields.
> 
> I initially had defined it in MachineFunction.h and then decided to duplicate the code in the two places it is required as per @tmsriram .
> 
> I now made the MIRParser's version and external function. Do you have a better suggestion?
> 
Oh, hmm, didn't follow the original conversation.  I don't really understand the objection from @tmsriram to making it a member function; 10 lines with tricky boundary conditions is generally beyond my threshold of "copy-paste it".


================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:556
+// This function has a linear time complexity.
 const MachineBasicBlock *MachineBasicBlock::getSectionEndMBB() const {
   const MachineFunction *MF = getParent();
----------------
getSectionEndMBB is dead?


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

https://reviews.llvm.org/D76954