[PATCH] [PeepholeOptimizer] Refactor the advance copy optimization to take advantage of the isRegSequence property

hfinkel at anl.gov hfinkel at anl.gov
Tue Aug 19 12:54:47 PDT 2014


================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:238
@@ +237,3 @@
+    /// If \p Reg is a physical register, a value tracker constructed with
+    /// this constructor will not find any alternative source.
+    ValueTracker(unsigned Reg, unsigned DefSubReg,
----------------
I would really like a more-verbose comment here (maybe with an example) to explain what this means. And, perhaps, why you might want to find an alternative source for a physical register or not.

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:533
@@ +532,3 @@
+  // So far we do not have any motivating example for doing that.
+  // Thus, instead of maintaining dead code, we will revisit that if that
+  // change at some point.
----------------
dead -> untested

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:534
@@ +533,3 @@
+  // Thus, instead of maintaining dead code, we will revisit that if that
+  // change at some point.
+  if (TargetRegisterInfo::isPhysicalRegister(Reg))
----------------
change -> changes

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:598
@@ +597,3 @@
+  /// Each call of this method moves the current source to the next
+  /// rewritable source.
+  /// \return True if a rewritable source has been found, false otherwise.
----------------
I don't understand this comment (and/or the associated logic below). When this is called we set CurrentSrcIdx = 1, and then we exit early if CurrentSrcIdx == 1. A comment on how this all works would be nice.

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:811
@@ +810,3 @@
+/// Two register classes are considered to be compatible if they share
+/// the same register bank.
+/// New copies issued by this optimization are register allocator
----------------
What code below enforces this "register bank" constraint?

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:851
@@ +850,3 @@
+  // We do not do that for now, because the intermediate from is
+  // currently not supported and we would need to add some book keeping
+  // to revert the unsupported patterns if the clean-up is not possible.
----------------
What exactly is not supported?

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:1264
@@ -894,2 +1263,3 @@
 
   // #1 Check if the inserted register matches the require sub index.
+  if (InsertedReg.SubIdx == DefSubReg) {
----------------
require -> required

http://reviews.llvm.org/D4874






More information about the llvm-commits mailing list