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

[Wei Mi via llvm-commits]

wmi 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) {
----------------
reames wrote:
> 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.
I see. An idea is at the second iteration, don't have to restart the index i from the first use operand. we can start from "the last value of i in the previous iteration" + 1, i.e., keep i monotonically increasing from one iteration to the next, so the complexity is O(n). 

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:5348
@@ -5340,2 +5347,3 @@
   }
+#endif
 
----------------
reames wrote:
> 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.
In X86InstrInfo::foldMemoryOperandImpl, if the opcode of current instruction is not in the table pointed by OpcodeTablePtr, it means it is impossible to get a valid instruction after folding, and foldMemoryOperandImpl will return nullptr. Like MemoryFoldTable1 specifies the set of instructions which can have their operand1 replaced by a mem operand. 


https://reviews.llvm.org/D24103