[PATCH] D20027: [RegAlloc, NFC] Extract LastSplitPoint computation to a new class

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Fri May 6 12:01:19 PDT 2016


MatzeB added inline comments.

================
Comment at: lib/CodeGen/SplitKit.h:41-44
@@ -40,1 +40,6 @@
 
+/// Analyze the last point in BB to insert split with respect to current
+/// LiveInterval. HoistSpillHelper also depends on the same analysis to find
+/// the last point to insert spill.
+class LLVM_LIBRARY_VISIBILITY LSplitPointAnalysis {
+private:
----------------
This explanation of the purpose of this class is confusing. This is not about liverange splitting (liverange splitting just happens to be one of the users later). The analysis simply determines the latest point in a block in which we can insert an instruction (that reads CurLI).

================
Comment at: lib/CodeGen/SplitKit.h:47
@@ +46,3 @@
+  const MachineFunction &MF;
+  const VirtRegMap &VRM;
+  const LiveIntervals &LIS;
----------------
VirtRegMap is not used here.

================
Comment at: lib/CodeGen/SplitKit.h:50
@@ +49,3 @@
+
+  // Current LiveInterval to split or spill.
+  const LiveInterval *CurLI;
----------------
Three slashes for doxygen comments.

================
Comment at: lib/CodeGen/SplitKit.h:61
@@ +60,3 @@
+
+  void setInterval(const LiveInterval *LI) { CurLI = LI; }
+
----------------
Use `const LiveInterval&` in the API, as nullptr is not allowed here.

================
Comment at: lib/CodeGen/SplitKit.h:63
@@ +62,3 @@
+
+  SlotIndex computeLastSplitPoint(unsigned Num);
+
----------------
I'd suggest using `const MachineBasicBlock&` instead of block numbers in the API here. That is simpler to use and you can get away without storing a reference to the MachineFunction in the LSplitPointAnalysis class.

================
Comment at: lib/CodeGen/SplitKit.h:76
@@ +75,3 @@
+  /// getLastSplitPointIter - Returns the last split point as an iterator.
+  MachineBasicBlock::iterator getLastSplitPointIter(MachineBasicBlock*);
+};
----------------
Use `const MachineBasicBlock&` in the API as nullptr is not allowed here.

================
Comment at: lib/CodeGen/SplitKit.h:218
@@ -197,1 +217,3 @@
   bool shouldSplitSingleBlock(const BlockInfo &BI, bool SingleInstrs) const;
+
+
----------------
no need for two empty lines.


Repository:
  rL LLVM

http://reviews.llvm.org/D20027





More information about the llvm-commits mailing list