[PATCH] D24103: [WIP][peephole] Enhance folding logic to work for STATEPOINTs

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:1683
@@ +1682,3 @@
+        // Note: This is O(n^2) in the number of operands if
+        // TII->optimizeLoadInstr were to fail for every other operand.
+        while(true) {
----------------
wmi wrote:
> Why recursive folding is needed here? I guess it is trying to fold the case that the address of a load is another load? If that is true, can we put the use operands of newly folded load into a set and only visit the set in the next iteration?
The case I'm trying to handle is a single instruction which can fold multiple distinct loads into it.  The problem is that when I replace MI, I need to restart the operand iteration.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:5336
@@ -5334,2 +5335,3 @@
+  // opposed to a set of FoldAsLoadDefs?  Can we remove?
   if (FoldAsLoadDefReg == 0)
     return nullptr;
----------------
wmi wrote:
> Looks like FoldAsLoadDefReg will never be 0 here.
That's what I thought, glad you agree.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:5348
@@ -5340,2 +5347,3 @@
   }
+#endif
 
----------------
wmi wrote:
> If MI is load, foldMemoryOperand will return nullptr in the end after it goes through a lot of effort.  I agree with you it is better to remove the check here and add an early exit in foldMemoryOperand.
It will?  I don't see why a instruction which can load definitely can't fold another load into itself.  An instruction set with an "reg = add mem, mem" instruction would be a great example.  Just because an "reg = add mem, reg" form loads doesn't mean you're done folding.


https://reviews.llvm.org/D24103