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

[Denis Antrushin via Phabricator via llvm-commits]

dantrushin added a comment.

F12488999: peephole.mir <https://reviews.llvm.org/F12488999>
Attached test will explode if `isTied` check is removed as suggested. 
Essential part is:

  %10:gr64 = MOV64rm killed %9, 1, $noreg, 0, $noreg :: (load 8 from %ir.p0, addrspace 1)
  %6:gr64, %7:gr64 = STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, %8, %8(tied-def 0), killed %10, %0(tied-def 1), ...

It is synthetic (i.e. we do no generate such MIR from any IR), but still not invalid



================
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.
----------------
reames wrote:
> 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.
> 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.

The possible path is:

PeepholeOptimizer::runOnMachineFunction (!isLoadFoldable() branch) ->
X86InstrInfo::optimizeLoadInstr ->
TargetInstrInfo::foldMemoryOperand ->
TargetInstrInfo::foldPatchpoint (here)

The fact this path is not exercised now is implementation detail (it did with earlier versions of ISEL patch).
I think we should keep this check to stay on the safe side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81646