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

Sriraman Tallam via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 9 17:39:25 PDT 2020


tmsriram added inline comments.


================
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();
----------------
efriedma wrote:
> 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".
I agree, Eli's recommendation is correct. 


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

https://reviews.llvm.org/D76954





More information about the llvm-commits mailing list