[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