[AArch64] Don't drop MMOs in the load/store optimizer when forming ldp/stp instructions or pre-/post-index loads/stores.

[mcrosier]

Summary

Without this change I believe the mi scheduler treats these transformed loads/stores as scheduling barriers. Therefore, I don't think we want to just drop the MMOs.

Chad

[gberry]

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

1269

For consistency, could you change this to drop the "MIB->" and just build off the above expr as was done in the change above?

1279

Ditto

[mcrosier]

Address Geoff's comments.

[flyingforyou]

LGTM!

[mcrosier]

Looks like this change is exposing a correctness issue in the MI scheduler. Investigating..

[mcrosier]

Per Junmo. I'm going to defer the part that adds the MMOs to the unscaled pairs until D8705 lands.

[gberry]

Can't all the other added memref cases also potentially run into the scheduler bug you're seeing?

[mcrosier]

In D16369#338363, @gberry wrote:

Can't all the other added memref cases also potentially run into the scheduler bug you're seeing?

It's not a problem in this case because we're just copying a single MMO from the ldr/str to the pre-/post-index equivalent. We run into problems only with the paired instructions where we have multiple MMOs.

[mcrosier]

Committed r259052.

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

if (isNarrowStore(Opc)) {
     // Handle Unscaled
     if (IsUnscaled)
       OffsetImm /= OffsetStride;
     MIB = BuildMI(*I->getParent(), InsertionPoint, I->getDebugLoc(),
                   TII->get(NewOpc))
               .addOperand(getLdStRegOp(RtMI))
               .addOperand(getLdStRegOp(Rt2MI))
               .addOperand(BaseRegOp)
-              .addImm(OffsetImm);
+              .addImm(OffsetImm)
+              .setMemRefs(I->mergeMemRefsWith(*Paired));
   }
 
   (void)MIB;
 
   // FIXME: Do we need/want to copy the mem operands from the source
   //        instructions? Probably. What uses them after this?
 
   DEBUG(dbgs() << "Creating pair load/store. Replacing instructions:\n    ");
AArch64LoadStoreOpt::mergeUpdateInsn(MachineBasicBlock::iterator I,
   MachineInstrBuilder MIB;
   if (!isPairedLdSt(I)) {
     // Non-paired instruction.
     MIB = BuildMI(*I->getParent(), I, I->getDebugLoc(), TII->get(NewOpc))
               .addOperand(getLdStRegOp(Update))
               .addOperand(getLdStRegOp(I))
               .addOperand(getLdStBaseOp(I))
               .addImm(Value);
+    MIB->setMemRefs(I->memoperands_begin(), I->memoperands_end());
   } else {
     // Paired instruction.
     int Scale = getMemScale(I);
     MIB = BuildMI(*I->getParent(), I, I->getDebugLoc(), TII->get(NewOpc))
               .addOperand(getLdStRegOp(Update))
               .addOperand(getLdStRegOp(I, 0))
               .addOperand(getLdStRegOp(I, 1))
               .addOperand(getLdStBaseOp(I))
               .addImm(Value / Scale);
+    MIB->setMemRefs(I->memoperands_begin(), I->memoperands_end());
   }
   (void)MIB;
 
   if (IsPreIdx)
     DEBUG(dbgs() << "Creating pre-indexed load/store.");
   else
     DEBUG(dbgs() << "Creating post-indexed load/store.");
   DEBUG(dbgs() << "    Replacing instructions:\n    ");