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

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:421
+  // Set the basic block that begins or ends every section.  For unique
+  // sections, the same basic block begins and ends it.
+  MachineBasicBlock *PrevMBB = nullptr;
----------------
tmsriram wrote:
> davidxl wrote:
> > It is not clear from the code below that the same BB begins and ends it for unique sections.
> For unique sections, this comes already set above in line 388 and 389.
ok.  Perhaps add a comment or ASSERT here.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:774
+  }
+  return getContext().getELFSection(Name, ELF::SHT_PROGBITS, Flags, 0,
+                                    GroupName, UniqueID);
----------------
Add a comment on '0' parameter :  0 /*blah ..*/,


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:786
+
+  if (!TM.getUniqueSectionNames()) {
+    Name += ".";
----------------
Is the logic reversed? Can you add a comment in the source?


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:807
+    const TargetMachine &TM) const {
+  SmallString<128> Name;
+  Name = (static_cast<MCSectionELF *>(MBB.getParent()->getSection()))
----------------
Why there is no getUniqueSectioNames check here as in other cases?


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

https://reviews.llvm.org/D73674