[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    ");
unsigned NewOpc = IsPreIdx ? getPreIndexedOpcode(I->getOpcode())
                              : getPostIndexedOpcode(I->getOpcode());
   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);
+              .addImm(Value)
+              .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);
+              .addImm(Value / Scale)
+              .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    ");

test/CodeGen/AArch64/aarch64-a57-fp-load-balancing.ll

-; RUN: llc < %s -mcpu=cortex-a57 -aarch64-a57-fp-load-balancing-override=1 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A57 --check-prefix CHECK-EVEN
-; RUN: llc < %s -mcpu=cortex-a57 -aarch64-a57-fp-load-balancing-override=2 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A57 --check-prefix CHECK-ODD
-; RUN: llc < %s -mcpu=cortex-a53 -aarch64-a57-fp-load-balancing-override=1 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A53 --check-prefix CHECK-EVEN
-; RUN: llc < %s -mcpu=cortex-a53 -aarch64-a57-fp-load-balancing-override=2 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A53 --check-prefix CHECK-ODD
+; RUN: llc < %s -mcpu=cortex-a57 -enable-misched=false -enable-post-misched=false -aarch64-a57-fp-load-balancing-override=1 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A57 --check-prefix CHECK-EVEN
+; RUN: llc < %s -mcpu=cortex-a57 -enable-misched=false -enable-post-misched=false -aarch64-a57-fp-load-balancing-override=2 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A57 --check-prefix CHECK-ODD
+; RUN: llc < %s -mcpu=cortex-a53 -enable-misched=false -enable-post-misched=false -aarch64-a57-fp-load-balancing-override=1 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A53 --check-prefix CHECK-EVEN
+; RUN: llc < %s -mcpu=cortex-a53 -enable-misched=false -enable-post-misched=false -aarch64-a57-fp-load-balancing-override=2 -aarch64-a57-fp-load-balancing-force-all | FileCheck %s --check-prefix CHECK --check-prefix CHECK-A53 --check-prefix CHECK-ODD
 
 ; Test the AArch64A57FPLoadBalancing pass. This pass relies heavily on register allocation, so
 ; our test strategy is to:
 ;   * Force the pass to always perform register swapping even if the dest register is of the
 ;     correct color already (-force-all)
 ;   * Force the pass to ignore all hints it obtained from regalloc (-deterministic-balance),
 ;     and run it twice, once where it always hints odd, and once where it always hints even.
 ;