[PATCH] D81646: MIR Statepoint refactoring. Part 2: Operand folding.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

LGTM.  I sat down today to write a cleaner patch, and in the process convinced myself this was in fact correct.  I will post a separate cleanup once this lands as I think we can have the foldMemoryOperand impl structured much more cleanly for tied operands, but doing so involves a slightly deeper rewrite of some x86 specific code than is reasonable to ask for here.

Please address the inline comments in this review when landing.

I would also encourage - but not require - you to fix 46916 in a separate patch before landing this.  I see that the tied argument propagation herein should fix that, but splitting the patch into "fold deopt w/vregs present" and "fold vreg gc operand" would make it much easier for anyone following along.



================
Comment at: llvm/lib/CodeGen/InlineSpiller.cpp:813
 
+  bool UntieRegs = MI->getOpcode() == TargetOpcode::STATEPOINT;
+
----------------
Please add a comment explaining assumption here.  


================
Comment at: llvm/lib/CodeGen/TargetInstrInfo.cpp:502
   for (unsigned Op : Ops) {
-    if (Op < StartIdx)
+    if (Op < NumDefs)
+      DefToFoldIdx = Op;
----------------
Please add an assert that DefToFoldIdx == e.  That is, there's at most one def op per fold operation.  The code below would miscompile a multiple def fold.  


================
Comment at: llvm/lib/CodeGen/TargetInstrInfo.cpp:506
+      return nullptr;
+    // When called from regalloc (InlineSpiller), operands must be untied,
+    // and regalloc will take care of (re)loading operand from memory.
----------------
I don't believe this line or comment are necessary.  Please remove if make check passes; if not, I will handle in follow up.

optimizeMemoryInst (the peephole entry point) rejects all defs, not just tied defs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81646/new/

https://reviews.llvm.org/D81646