[PATCH] D83885: [Propeller]: Use a descriptive temporary symbol name for the end of the basic block.

Sriraman Tallam via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 15 22:54:21 PDT 2020


tmsriram added a comment.

Some inital comments.  Expanding the summary to provide a bit more detail on why this symbol naming scheme is better might be useful.  Thanks for doing this.



================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:177
 
-  /// Used during basic block sections to mark the end of a basic block.
-  MCSymbol *EndMCSymbol = nullptr;
+  /// Marks the end of the basic block, to be used to calculate the size of this
+  /// basic block, or the basic block section ending with it.
----------------
The comment "Used during basic block sections..." is still relevant?


================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:478
 
+  MCSymbol *getEndSymbol() const;
+
----------------
Can we  name it getCachedEndMCSymbol?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3055
+  // Check if this is an unreachable MBB requiring a section.
+  bool UnreachableMBBSection =
+      MBB.pred_empty() && (&MBB != &this->MF->front()) && MBB.isBeginSection();
----------------
Remove this, it does not belong to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83885





More information about the llvm-commits mailing list