[x86] Propagate memory operands during call frame optimization

[Kayjukh]

Summary

Propagate memory operands when folding load instructions into instructions that directly operate on memory.

The original revision has been split. See D80140 for the other part of the changes.

[craig.topper]

The change to X86ISelDAGToDAG.cpp looks fine. I have questions about the other change.

llvm/lib/Target/X86/X86CallFrameOptimization.cpp

565

This captures the memoperand for the load of the source, but what about the store to the stack?

571

This also has a store to stack

[Kayjukh]

llvm/lib/Target/X86/X86CallFrameOptimization.cpp

565

I'm not really sure how to model this. As far as I can tell, we don't know the location to which the value gets stored on the stack. Would you add a memory operand with the MachinePointerInfo returned by MachinePointerInfo::getUnknownStack?
Or am I missing something?

571

Same comment as above.

[craig.topper]

llvm/lib/Target/X86/X86CallFrameOptimization.cpp

565

Does the Store node that we're replacing have a mem operand?

[Kayjukh]

Also propagate store memory operands during call frame optimization.

[Kayjukh]

llvm/lib/Target/X86/X86CallFrameOptimization.cpp

565

Yes, I overlooked the store. Should be good now!

[lebedev.ri]

Can there be test coverage for this?

[craig.topper]

In D80062#2040256, @lebedev.ri wrote:

Can there be test coverage for this?

For the X86ISelDAGToDAG.cpp change it should be possible to do -stop-after=finalize-isel and inspect the MIR output.

For the out we should be able to have .mir input that just runs the call frame optimization pass.

We should split these two changes into two reviews.

[Kayjukh]

Split the call frame optimization patch from the ISel DAG postprocessing patch.

[efriedma]

Code looks reasonable. It would be nice to have an MIR testcase showing the memoperands after this pass runs, even if there isn't any change to the final asm without additional work.

[Kayjukh]

Also propagate memory operands when folding non-MOV instructions. Add a test case.

[craig.topper]

LGTM

llvm/lib/Target/X86/X86CallFrameOptimization.cpp

case X86::MOV64mr: {
       MachineInstr *DefMov = nullptr;
       if (!SlowPUSHrmm && (DefMov = canFoldIntoRegPush(FrameSetup, Reg))) {
         PushOpcode = Is64Bit ? X86::PUSH64rmm : X86::PUSH32rmm;
         Push = BuildMI(MBB, Context.Call, DL, TII->get(PushOpcode));
 
         unsigned NumOps = DefMov->getDesc().getNumOperands();
         for (unsigned i = NumOps - X86::AddrNumOperands; i != NumOps; ++i)
           Push->addOperand(DefMov->getOperand(i));
+        Push->cloneMergedMemRefs(MF, {&*DefMov, &*Store});
 
         DefMov->eraseFromParent();
       } else {
         PushOpcode = Is64Bit ? X86::PUSH64r : X86::PUSH32r;
         Push = BuildMI(MBB, Context.Call, DL, TII->get(PushOpcode))
                    .addReg(Reg)
                    .getInstr();
+        Push->cloneMemRefs(MF, *Store);
       }
       break;
     }
     }
 
     // For debugging, when using SP-based CFA, we need to adjust the CFA
     // offset after each push.
     // TODO: This is needed only if we require precise CFA.

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

if ((Opc == X86::TEST8rr || Opc == X86::TEST16rr ||
                           And.getOperand(2),
                           And.getOperand(3),
                           And.getOperand(4),
                           And.getOperand(5),
                           And.getOperand(0),
                           And.getOperand(6)  /* Chain */ };
         MachineSDNode *Test = CurDAG->getMachineNode(NewOpc, SDLoc(N),
                                                      MVT::i32, MVT::Other, Ops);
+        CurDAG->setNodeMemRefs(
+            Test, cast<MachineSDNode>(And.getNode())->memoperands());
         ReplaceUses(N, Test);
         MadeChange = true;
         continue;
       }
     }
 
     // Look for a KAND+KORTEST and turn it into KTEST if only the zero flag is
     // used. We're doing this late so we can prefer to fold the AND into masked