[MachineScheduler] Order more stores by ascending address

[Allen]

Summary

According D125377, we order STP Q's by ascending address. While on some
targets, paired 128 bit loads and stores are slow, so the STP will split
into STRQ and STUR, so I hope these stores will also be ordered.
Also add subtarget feature ascend-store-address to control the aggressive order.

[Allen]

This comment was removed by Allen.

[dmgreen]

Do you have more details on where and when you expect this to be beneficial? It may not look like it from the review, but we did a fair amount of testing and benchmarking was done on D125377 on all kinds of CPUs (in-order vs out-of-order, little vs big, etc) to make sure that it was an improvement or benign on the cases we tried. I don't think I would be against this - so long as we had a decent reason to do so. It does constrain the scheduling graph though, so we shouldn't do so unnecessarily.

[Allen]

In D126700#3559712, @dmgreen wrote:

Do you have more details on where and when you expect this to be beneficial? It may not look like it from the review, but we did a fair amount of testing and benchmarking was done on D125377 on all kinds of CPUs (in-order vs out-of-order, little vs big, etc) to make sure that it was an improvement or benign on the cases we tried. I don't think I would be against this - so long as we had a decent reason to do so. It does constrain the scheduling graph though, so we shouldn't do so unnecessarily.

Thanks @dmgreen, base on the following test case , I'll gain benefits base on the tsv110 target (kunpeng 920)
BTW：I have another version, which only do some refactor(keep the logic unchanged), as we don't add new store instructions STRQ and STUR, see detail on https://reviews.llvm.org/D126700?id=433075, which is better ?

double array[0x8000];

void foo (double array[], int size) {
  for (int i=0; i<size; i++) {
     array[i] = 2.0;
  }
  return;
}

int main (){
   
  for (int i=0; i<100000; i++) {
    foo (array, sizeof (array)/sizeof (array[0]));  
  }
  return 0;
}

[dmgreen]

OK I see, thanks. In this version on godbolt I only see ordered STPQ's, perhaps you have some downstream differences that would alter the codegen?
https://godbolt.org/z/1nbr7Eh8r

With this kind of thing we always have two options. Either we can do it unconditionally for all cpus, or we can add a subtarget tuning feature to steer the codegen for specific cpus. There are a number already like FeatureFuseAES that control fusing in the machine scheduler. Adding another to control a more aggressive ordering in AArch64PostRASchedStrategy sounds perfectly sensible to me.

The other option is to do this for all cpus. The general rule there is it should be something that we expect to be beneficial or benign on all/most cpus that would run -mcpu=generic code. I'm not strongly against that for this patch, we need to check that with benchmarks and I did see a case in testing which changed the code from:

STR      q1,[sp,#0x40]     
LDP      q1,q2,[x25,#-0x10]
STR      q0,[sp,#0x60]     
LDUR     q0,[x24,#-0x10]   
FDIV     v0.2D,v0.2D,v1.2D 
STP      q2,q1,[sp,#0x20]  
...
=>
STR      q1,[sp,#0x40]     
LDP      q1,q2,[x25,#-0x10]
STP      q2,q1,[sp,#0x20]  
STR      q0,[sp,#0x60]     
LDUR     q0,[x24,#-0x10]   
FDIV     v0.2D,v0.2D,v1.2D 
...

Those clumped-together stores were apparently a little slower. Perhaps it was just unlucky, and it is only one case. I would guess that in-order memory operations are a little easier for prefetchers than ones that bobble around, but I don't have any evidence of that, and prefetchers are usually pretty sophisticated nowadays. It would take a complex case for something like that to matter, and complex cases can easily defy expectation.

So - do you have any other benchmark results, on any other cpus? Or as an alternative, do you think that adding a subtarget feature is an OK solution?

[Allen]

In D126700#3564241, @dmgreen wrote:

OK I see, thanks. In this version on godbolt I only see ordered STPQ's, perhaps you have some downstream differences that would alter the codegen?
So - do you have any other benchmark results, on any other cpus? Or as an alternative, do you think that adding a subtarget feature is an OK solution?

Many thanks @dmgreen for your detailed reply, and I found the newest upstream already sort the stores by ascending address for tsv110 target. while on my downstream, the assemble is a bit similar with cortex-a57 target. https://godbolt.org/z/bWeKq59ha

So it seems add a subtarget feature is a more reasonable, I'll try that.

[Allen]

Also add subtarget feature ascend-store-address to control the aggressive order.

[fhahn]

A subtarget feature sounds like a good way to gate this, thanks! It makes sense to only do this for CPUs where it's proven to be profitable IMO.

[dmgreen]

llvm/lib/Target/AArch64/AArch64.td

219

Capitalize Schedule. Maybe reword as "Schedule vector stores by ascending address".

llvm/lib/Target/AArch64/AArch64InstrInfo.h

106

Can you add a /// doc string

504–505

I think you can probably move these into AArch64InstrInfo, near to isPairedLdSt

llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp

29

Why do we need to check it is an immediate?

[Allen]

Adress review comment, thanks @dmgreen @fhahn

[dmgreen]

Thanks for the update

llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp

29

Why is this needed?

[Allen]

llvm/lib/Target/AArch64/AArch64.td

219

Thanks, apply your comment.

llvm/lib/Target/AArch64/AArch64InstrInfo.h

504–505

Done, thanks!

llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp

29

this is because some IR used to match the str q0, [x8, :lo12:seed_lo] for example, and we can't get its offset

29

if we don't check this, it will crash to use getImm() get the immediate with the above str q0, [x8, :lo12:seed_lo]

[dmgreen]

Thanks. LGTM

llvm/lib/Target/AArch64/AArch64.td

219

A q store is a store of a vector. This subtarget feature doesn't alter the scalar scores like x/w/s.

llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp

29

OK I see, it is a TargetFlags. That makes sense. It doesn't look like we ever generate them for STQP.

llvm/lib/Target/AArch64/AArch64.td

def FeaturePostRAScheduler : SubtargetFeature<"use-postra-scheduler",
     "UsePostRAScheduler", "true", "Schedule again after register allocation">;
 
 def FeatureSlowMisaligned128Store : SubtargetFeature<"slow-misaligned-128store",
     "IsMisaligned128StoreSlow", "true", "Misaligned 128 bit stores are slow">;
 
 def FeatureSlowPaired128 : SubtargetFeature<"slow-paired-128",
     "IsPaired128Slow", "true", "Paired 128 bit loads and stores are slow">;
 
+def FeatureAscendStoreAddress : SubtargetFeature<"ascend-store-address",
+    "IsStoreAddressAscend", "false",
+    "Schedule scalar stores by ascending address">;
+
 def FeatureSlowSTRQro : SubtargetFeature<"slow-strqro-store", "IsSTRQroSlow",
     "true", "STR of Q register with register offset is slow">;
 
 def FeatureAlternateSExtLoadCVTF32Pattern : SubtargetFeature<
     "alternate-sextload-cvt-f32-pattern", "UseAlternateSExtLoadCVTF32Pattern",
     "true", "Use alternative pattern for sextload convert to f32">;
 
 def FeatureArithmeticBccFusion : SubtargetFeature<

llvm/lib/Target/AArch64/AArch64InstrInfo.h

public:
   static bool isPreLd(const MachineInstr &MI);
 
   /// Returns whether the instruction is a pre-indexed store.
   static bool isPreSt(const MachineInstr &MI);
 
   /// Returns whether the instruction is a pre-indexed load/store.
   static bool isPreLdSt(const MachineInstr &MI);
 
+  /// Returns whether the instruction is a paired load/store.
+  static bool isPairedLdSt(const MachineInstr &MI);
+
+  /// Returns the base register operator of a load/store.
+  static const MachineOperand &getLdStBaseOp(const MachineInstr &MI);
+
+  /// Returns the the immediate offset operator of a load/store.
+  static const MachineOperand &getLdStOffsetOp(const MachineInstr &MI);
+
   /// Returns whether the instruction is FP or NEON.
   static bool isFpOrNEON(const MachineInstr &MI);
 
   /// Returns whether the instruction is in Q form (128 bit operands)
   static bool isQForm(const MachineInstr &MI);
 
   /// Returns the index for the immediate for a given instruction.
   static unsigned getLoadStoreImmIdx(unsigned Opc);
case AArch64::PTRUE_D:
     return true;
   default:
     return false;
   }
 }
 
 /// Return opcode to be used for indirect calls.
 unsigned getBLRCallOpcode(const MachineFunction &MF);
 
 // struct TSFlags {
 #define TSFLAG_ELEMENT_SIZE_TYPE(X)      (X)       // 3-bits
 #define TSFLAG_DESTRUCTIVE_INST_TYPE(X) ((X) << 3) // 4-bits
 #define TSFLAG_FALSE_LANE_TYPE(X)       ((X) << 7) // 2-bits
 #define TSFLAG_INSTR_FLAGS(X)           ((X) << 9) // 2-bits
 // }
 
 namespace AArch64 {
 

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

case AArch64::STRQpre:
     return true;
   }
 }
 
 bool AArch64InstrInfo::isPreLdSt(const MachineInstr &MI) {
   return isPreLd(MI) || isPreSt(MI);
 }
 
+bool AArch64InstrInfo::isPairedLdSt(const MachineInstr &MI) {
+  switch (MI.getOpcode()) {
+  default:
+    return false;
+  case AArch64::LDPSi:
+  case AArch64::LDPSWi:
+  case AArch64::LDPDi:
+  case AArch64::LDPQi:
+  case AArch64::LDPWi:
+  case AArch64::LDPXi:
+  case AArch64::STPSi:
+  case AArch64::STPDi:
+  case AArch64::STPQi:
+  case AArch64::STPWi:
+  case AArch64::STPXi:
+  case AArch64::STGPi:
+    return true;
+  }
+}
+
+const MachineOperand &AArch64InstrInfo::getLdStBaseOp(const MachineInstr &MI) {
+  unsigned Idx =
+      AArch64InstrInfo::isPairedLdSt(MI) || AArch64InstrInfo::isPreLdSt(MI) ? 2
+                                                                            : 1;
+  return MI.getOperand(Idx);
+}
+
+const MachineOperand &
+AArch64InstrInfo::getLdStOffsetOp(const MachineInstr &MI) {
+  unsigned Idx =
+      AArch64InstrInfo::isPairedLdSt(MI) || AArch64InstrInfo::isPreLdSt(MI) ? 3
+                                                                            : 2;
+  return MI.getOperand(Idx);
+}
+
 static const TargetRegisterClass *getRegClass(const MachineInstr &MI,
                                               Register Reg) {
   if (MI.getParent() == nullptr)
     return nullptr;
   const MachineFunction *MF = MI.getParent()->getParent();
   return MF ? MF->getRegInfo().getRegClassOrNull(Reg) : nullptr;
 }
 

llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

case AArch64::ST2GOffset:
     return AArch64::ST2GPostIndex;
   case AArch64::STZ2GOffset:
     return AArch64::STZ2GPostIndex;
   case AArch64::STGPi:
     return AArch64::STGPpost;
   }
 }
 
-static bool isPairedLdSt(const MachineInstr &MI) {
-  switch (MI.getOpcode()) {
-  default:
-    return false;
-  case AArch64::LDPSi:
-  case AArch64::LDPSWi:
-  case AArch64::LDPDi:
-  case AArch64::LDPQi:
-  case AArch64::LDPWi:
-  case AArch64::LDPXi:
-  case AArch64::STPSi:
-  case AArch64::STPDi:
-  case AArch64::STPQi:
-  case AArch64::STPWi:
-  case AArch64::STPXi:
-  case AArch64::STGPi:
-    return true;
-  }
-}
-
 static bool isPreLdStPairCandidate(MachineInstr &FirstMI, MachineInstr &MI) {
 
   unsigned OpcA = FirstMI.getOpcode();
   unsigned OpcB = MI.getOpcode();
 
   switch (OpcA) {
   default:
     return false;
static bool isPreLdStPairCandidate(MachineInstr &FirstMI, MachineInstr &MI) {
   case AArch64::LDRXpre:
     return (OpcB == AArch64::LDRXui) || (OpcB == AArch64::LDURXi);
   }
 }
 
 // Returns the scale and offset range of pre/post indexed variants of MI.
 static void getPrePostIndexedMemOpInfo(const MachineInstr &MI, int &Scale,
                                        int &MinOffset, int &MaxOffset) {
-  bool IsPaired = isPairedLdSt(MI);
+  bool IsPaired = AArch64InstrInfo::isPairedLdSt(MI);
   bool IsTagStore = isTagStore(MI);
   // ST*G and all paired ldst have the same scale in pre/post-indexed variants
   // as in the "unsigned offset" variant.
   // All other pre/post indexed ldst instructions are unscaled.
   Scale = (IsTagStore || IsPaired) ? AArch64InstrInfo::getMemScale(MI) : 1;
 
   if (IsPaired) {
     MinOffset = -64;
     MaxOffset = 63;
   } else {
     MinOffset = -256;
     MaxOffset = 255;
   }
 }
 
 static MachineOperand &getLdStRegOp(MachineInstr &MI,
                                     unsigned PairedRegOp = 0) {
   assert(PairedRegOp < 2 && "Unexpected register operand idx.");
   bool IsPreLdSt = AArch64InstrInfo::isPreLdSt(MI);
   if (IsPreLdSt)
     PairedRegOp += 1;
-  unsigned Idx = isPairedLdSt(MI) || IsPreLdSt ? PairedRegOp : 0;
-  return MI.getOperand(Idx);
-}
-
-static const MachineOperand &getLdStBaseOp(const MachineInstr &MI) {
-  unsigned Idx = isPairedLdSt(MI) || AArch64InstrInfo::isPreLdSt(MI) ? 2 : 1;
-  return MI.getOperand(Idx);
-}
-
-static const MachineOperand &getLdStOffsetOp(const MachineInstr &MI) {
-  unsigned Idx = isPairedLdSt(MI) || AArch64InstrInfo::isPreLdSt(MI) ? 3 : 2;
+  unsigned Idx =
+      AArch64InstrInfo::isPairedLdSt(MI) || IsPreLdSt ? PairedRegOp : 0;
   return MI.getOperand(Idx);
 }
 
 static bool isLdOffsetInRangeOfSt(MachineInstr &LoadInst,
                                   MachineInstr &StoreInst,
                                   const AArch64InstrInfo *TII) {
   assert(isMatchingStore(LoadInst, StoreInst) && "Expect only matched ld/st.");
   int LoadSize = TII->getMemScale(LoadInst);
   int StoreSize = TII->getMemScale(StoreInst);
-  int UnscaledStOffset = TII->hasUnscaledLdStOffset(StoreInst)
-                             ? getLdStOffsetOp(StoreInst).getImm()
-                             : getLdStOffsetOp(StoreInst).getImm() * StoreSize;
-  int UnscaledLdOffset = TII->hasUnscaledLdStOffset(LoadInst)
-                             ? getLdStOffsetOp(LoadInst).getImm()
-                             : getLdStOffsetOp(LoadInst).getImm() * LoadSize;
+  int UnscaledStOffset =
+      TII->hasUnscaledLdStOffset(StoreInst)
+          ? AArch64InstrInfo::getLdStOffsetOp(StoreInst).getImm()
+          : AArch64InstrInfo::getLdStOffsetOp(StoreInst).getImm() * StoreSize;
+  int UnscaledLdOffset =
+      TII->hasUnscaledLdStOffset(LoadInst)
+          ? AArch64InstrInfo::getLdStOffsetOp(LoadInst).getImm()
+          : AArch64InstrInfo::getLdStOffsetOp(LoadInst).getImm() * LoadSize;
   return (UnscaledStOffset <= UnscaledLdOffset) &&
          (UnscaledLdOffset + LoadSize <= (UnscaledStOffset + StoreSize));
 }
 
 static bool isPromotableZeroStoreInst(MachineInstr &MI) {
   unsigned Opc = MI.getOpcode();
   return (Opc == AArch64::STRWui || Opc == AArch64::STURWi ||
           isNarrowStore(Opc)) &&
static bool isMergeableLdStUpdate(MachineInstr &MI) {
   case AArch64::LDPWi:
   case AArch64::LDPXi:
   case AArch64::STPSi:
   case AArch64::STPDi:
   case AArch64::STPQi:
   case AArch64::STPWi:
   case AArch64::STPXi:
     // Make sure this is a reg+imm (as opposed to an address reloc).
-    if (!getLdStOffsetOp(MI).isImm())
+    if (!AArch64InstrInfo::getLdStOffsetOp(MI).isImm())
       return false;
 
     return true;
   }
 }
 
 MachineBasicBlock::iterator
 AArch64LoadStoreOpt::mergeNarrowZeroStores(MachineBasicBlock::iterator I,
AArch64LoadStoreOpt::mergeNarrowZeroStores(MachineBasicBlock::iterator I,
 
   bool MergeForward = Flags.getMergeForward();
   // Insert our new paired instruction after whichever of the paired
   // instructions MergeForward indicates.
   MachineBasicBlock::iterator InsertionPoint = MergeForward ? MergeMI : I;
   // Also based on MergeForward is from where we copy the base register operand
   // so we get the flags compatible with the input code.
   const MachineOperand &BaseRegOp =
-      MergeForward ? getLdStBaseOp(*MergeMI) : getLdStBaseOp(*I);
+      MergeForward ? AArch64InstrInfo::getLdStBaseOp(*MergeMI)
+                   : AArch64InstrInfo::getLdStBaseOp(*I);
 
   // Which register is Rt and which is Rt2 depends on the offset order.
   MachineInstr *RtMI;
-  if (getLdStOffsetOp(*I).getImm() ==
-      getLdStOffsetOp(*MergeMI).getImm() + OffsetStride)
+  if (AArch64InstrInfo::getLdStOffsetOp(*I).getImm() ==
+      AArch64InstrInfo::getLdStOffsetOp(*MergeMI).getImm() + OffsetStride)
     RtMI = &*MergeMI;
   else
     RtMI = &*I;
 
-  int OffsetImm = getLdStOffsetOp(*RtMI).getImm();
+  int OffsetImm = AArch64InstrInfo::getLdStOffsetOp(*RtMI).getImm();
   // Change the scaled offset from small to large type.
   if (IsScaled) {
     assert(((OffsetImm & 1) == 0) && "Unexpected offset to merge");
     OffsetImm /= 2;
   }
 
   // Construct the new instruction.
   DebugLoc DL = I->getDebugLoc();
#endif
   }
 
   // Insert our new paired instruction after whichever of the paired
   // instructions MergeForward indicates.
   MachineBasicBlock::iterator InsertionPoint = MergeForward ? Paired : I;
   // Also based on MergeForward is from where we copy the base register operand
   // so we get the flags compatible with the input code.
   const MachineOperand &BaseRegOp =
-      MergeForward ? getLdStBaseOp(*Paired) : getLdStBaseOp(*I);
+      MergeForward ? AArch64InstrInfo::getLdStBaseOp(*Paired)
+                   : AArch64InstrInfo::getLdStBaseOp(*I);
 
-  int Offset = getLdStOffsetOp(*I).getImm();
-  int PairedOffset = getLdStOffsetOp(*Paired).getImm();
+  int Offset = AArch64InstrInfo::getLdStOffsetOp(*I).getImm();
+  int PairedOffset = AArch64InstrInfo::getLdStOffsetOp(*Paired).getImm();
   bool PairedIsUnscaled = TII->hasUnscaledLdStOffset(Paired->getOpcode());
   if (IsUnscaled != PairedIsUnscaled) {
     // We're trying to pair instructions that differ in how they are scaled.  If
     // I is scaled then scale the offset of Paired accordingly.  Otherwise, do
     // the opposite (i.e., make Paired's offset unscaled).
     int MemSize = TII->getMemScale(*Paired);
     if (PairedIsUnscaled) {
       // If the unscaled offset isn't a multiple of the MemSize, we can't
if (Offset == PairedOffset + OffsetStride &&
     // I.e., we turn ldp I, Paired into ldp Paired, I.
     // Update the index accordingly.
     if (SExtIdx != -1)
       SExtIdx = (SExtIdx + 1) % 2;
   } else {
     RtMI = &*I;
     Rt2MI = &*Paired;
   }
-  int OffsetImm = getLdStOffsetOp(*RtMI).getImm();
+  int OffsetImm = AArch64InstrInfo::getLdStOffsetOp(*RtMI).getImm();
   // Scale the immediate offset, if necessary.
   if (TII->hasUnscaledLdStOffset(RtMI->getOpcode())) {
     assert(!(OffsetImm % TII->getMemScale(*RtMI)) &&
            "Unscaled offset cannot be scaled.");
     OffsetImm /= TII->getMemScale(*RtMI);
   }
 
   // Construct the new instruction.
if (LoadSize == StoreSize && (LoadSize == 4 || LoadSize == 8)) {
     // FIXME: Currently we disable this transformation in big-endian targets as
     // performance and correctness are verified only in little-endian.
     if (!Subtarget->isLittleEndian())
       return NextI;
     bool IsUnscaled = TII->hasUnscaledLdStOffset(*LoadI);
     assert(IsUnscaled == TII->hasUnscaledLdStOffset(*StoreI) &&
            "Unsupported ld/st match");
     assert(LoadSize <= StoreSize && "Invalid load size");
-    int UnscaledLdOffset = IsUnscaled
-                               ? getLdStOffsetOp(*LoadI).getImm()
-                               : getLdStOffsetOp(*LoadI).getImm() * LoadSize;
-    int UnscaledStOffset = IsUnscaled
-                               ? getLdStOffsetOp(*StoreI).getImm()
-                               : getLdStOffsetOp(*StoreI).getImm() * StoreSize;
+    int UnscaledLdOffset =
+        IsUnscaled
+            ? AArch64InstrInfo::getLdStOffsetOp(*LoadI).getImm()
+            : AArch64InstrInfo::getLdStOffsetOp(*LoadI).getImm() * LoadSize;
+    int UnscaledStOffset =
+        IsUnscaled
+            ? AArch64InstrInfo::getLdStOffsetOp(*StoreI).getImm()
+            : AArch64InstrInfo::getLdStOffsetOp(*StoreI).getImm() * StoreSize;
     int Width = LoadSize * 8;
     Register DestReg =
         IsStoreXReg ? Register(TRI->getMatchingSuperReg(
                           LdRt, AArch64::sub_32, &AArch64::GPR64RegClass))
                     : LdRt;
 
     assert((UnscaledLdOffset >= UnscaledStOffset &&
             (UnscaledLdOffset + LoadSize) <= UnscaledStOffset + StoreSize) &&

 }
 
 bool AArch64LoadStoreOpt::findMatchingStore(
     MachineBasicBlock::iterator I, unsigned Limit,
     MachineBasicBlock::iterator &StoreI) {
   MachineBasicBlock::iterator B = I->getParent()->begin();
   MachineBasicBlock::iterator MBBI = I;
   MachineInstr &LoadMI = *I;
-  Register BaseReg = getLdStBaseOp(LoadMI).getReg();
+  Register BaseReg = AArch64InstrInfo::getLdStBaseOp(LoadMI).getReg();
 
   // If the load is the first instruction in the block, there's obviously
   // not any matching store.
   if (MBBI == B)
     return false;
 
   // Track which register units have been modified and used between the first
   // insn and the second insn.
do {
 
     // If the load instruction reads directly from the address to which the
     // store instruction writes and the stored value is not modified, we can
     // promote the load. Since we do not handle stores with pre-/post-index,
     // it's unnecessary to check if BaseReg is modified by the store itself.
     // Also we can't handle stores without an immediate offset operand,
     // while the operand might be the address for a global variable.
     if (MI.mayStore() && isMatchingStore(LoadMI, MI) &&
-        BaseReg == getLdStBaseOp(MI).getReg() && getLdStOffsetOp(MI).isImm() &&
+        BaseReg == AArch64InstrInfo::getLdStBaseOp(MI).getReg() &&
+        AArch64InstrInfo::getLdStOffsetOp(MI).isImm() &&
         isLdOffsetInRangeOfSt(LoadMI, MI, TII) &&
         ModifiedRegUnits.available(getLdStRegOp(MI).getReg())) {
       StoreI = MBBI;
       return true;
     }
 
     if (MI.isCall())
       return false;
AArch64LoadStoreOpt::findMatchingInsn(MachineBasicBlock::iterator I,
   MachineBasicBlock::iterator MBBI = I;
   MachineBasicBlock::iterator MBBIWithRenameReg;
   MachineInstr &FirstMI = *I;
   MBBI = next_nodbg(MBBI, E);
 
   bool MayLoad = FirstMI.mayLoad();
   bool IsUnscaled = TII->hasUnscaledLdStOffset(FirstMI);
   Register Reg = getLdStRegOp(FirstMI).getReg();
-  Register BaseReg = getLdStBaseOp(FirstMI).getReg();
-  int Offset = getLdStOffsetOp(FirstMI).getImm();
+  Register BaseReg = AArch64InstrInfo::getLdStBaseOp(FirstMI).getReg();
+  int Offset = AArch64InstrInfo::getLdStOffsetOp(FirstMI).getImm();
   int OffsetStride = IsUnscaled ? TII->getMemScale(FirstMI) : 1;
   bool IsPromotableZeroStore = isPromotableZeroStoreInst(FirstMI);
 
   Optional<bool> MaybeCanRename = None;
   if (!EnableRenaming)
     MaybeCanRename = {false};
 
   SmallPtrSet<const TargetRegisterClass *, 5> RequiredClasses;
for (unsigned Count = 0; MBBI != E && Count < Limit;
 
     // Don't count transient instructions towards the search limit since there
     // may be different numbers of them if e.g. debug information is present.
     if (!MI.isTransient())
       ++Count;
 
     Flags.setSExtIdx(-1);
     if (areCandidatesToMergeOrPair(FirstMI, MI, Flags, TII) &&
-        getLdStOffsetOp(MI).isImm()) {
+        AArch64InstrInfo::getLdStOffsetOp(MI).isImm()) {
       assert(MI.mayLoadOrStore() && "Expected memory operation.");
       // If we've found another instruction with the same opcode, check to see
       // if the base and offset are compatible with our starting instruction.
       // These instructions all have scaled immediate operands, so we just
       // check for +1/-1. Make sure to check the new instruction offset is
       // actually an immediate and not a symbolic reference destined for
       // a relocation.
-      Register MIBaseReg = getLdStBaseOp(MI).getReg();
-      int MIOffset = getLdStOffsetOp(MI).getImm();
+      Register MIBaseReg = AArch64InstrInfo::getLdStBaseOp(MI).getReg();
+      int MIOffset = AArch64InstrInfo::getLdStOffsetOp(MI).getImm();
       bool MIIsUnscaled = TII->hasUnscaledLdStOffset(MI);
       if (IsUnscaled != MIIsUnscaled) {
         // We're trying to pair instructions that differ in how they are scaled.
         // If FirstMI is scaled then scale the offset of MI accordingly.
         // Otherwise, do the opposite (i.e., make MI's offset unscaled).
         int MemSize = TII->getMemScale(MI);
         if (MIIsUnscaled) {
           // If the unscaled offset isn't a multiple of the MemSize, we can't
if (areCandidatesToMergeOrPair(FirstMI, MI, Flags, TII) &&
 
       if (BaseReg == MIBaseReg) {
         // If the offset of the second ld/st is not equal to the size of the
         // destination register it can’t be paired with a pre-index ld/st
         // pair. Additionally if the base reg is used or modified the operations
         // can't be paired: bail and keep looking.
         if (IsPreLdSt) {
           bool IsOutOfBounds = MIOffset != TII->getMemScale(MI);
-          bool IsBaseRegUsed =
-              !UsedRegUnits.available(getLdStBaseOp(MI).getReg());
-          bool IsBaseRegModified =
-              !ModifiedRegUnits.available(getLdStBaseOp(MI).getReg());
+          bool IsBaseRegUsed = !UsedRegUnits.available(
+              AArch64InstrInfo::getLdStBaseOp(MI).getReg());
+          bool IsBaseRegModified = !ModifiedRegUnits.available(
+              AArch64InstrInfo::getLdStBaseOp(MI).getReg());
           // If the stored value and the address of the second instruction is
           // the same, it needs to be using the updated register and therefore
           // it must not be folded.
-          bool IsMIRegTheSame = TRI->regsOverlap(getLdStRegOp(MI).getReg(),
-                                                 getLdStBaseOp(MI).getReg());
+          bool IsMIRegTheSame =
+              TRI->regsOverlap(getLdStRegOp(MI).getReg(),
+                               AArch64InstrInfo::getLdStBaseOp(MI).getReg());
           if (IsOutOfBounds || IsBaseRegUsed || IsBaseRegModified ||
               IsMIRegTheSame) {
             LiveRegUnits::accumulateUsedDefed(MI, ModifiedRegUnits,
                                               UsedRegUnits, TRI);
             MemInsns.push_back(&MI);
             continue;
           }
         } else {

 
 static MachineBasicBlock::iterator
 maybeMoveCFI(MachineInstr &MI, MachineBasicBlock::iterator MaybeCFI) {
   auto End = MI.getParent()->end();
   if (MaybeCFI == End ||
       MaybeCFI->getOpcode() != TargetOpcode::CFI_INSTRUCTION ||
       !(MI.getFlag(MachineInstr::FrameSetup) ||
         MI.getFlag(MachineInstr::FrameDestroy)) ||
-      getLdStBaseOp(MI).getReg() != AArch64::SP)
+      AArch64InstrInfo::getLdStBaseOp(MI).getReg() != AArch64::SP)
     return End;
 
   const MachineFunction &MF = *MI.getParent()->getParent();
   unsigned CFIIndex = MaybeCFI->getOperand(0).getCFIIndex();
   const MCCFIInstruction &CFI = MF.getFrameInstructions()[CFIIndex];
   switch (CFI.getOperation()) {
   case MCCFIInstruction::OpDefCfa:
   case MCCFIInstruction::OpDefCfaOffset:
AArch64LoadStoreOpt::mergeUpdateInsn(MachineBasicBlock::iterator I,
   if (Update->getOpcode() == AArch64::SUBXri)
     Value = -Value;
 
   unsigned NewOpc = IsPreIdx ? getPreIndexedOpcode(I->getOpcode())
                              : getPostIndexedOpcode(I->getOpcode());
   MachineInstrBuilder MIB;
   int Scale, MinOffset, MaxOffset;
   getPrePostIndexedMemOpInfo(*I, Scale, MinOffset, MaxOffset);
-  if (!isPairedLdSt(*I)) {
+  if (!AArch64InstrInfo::isPairedLdSt(*I)) {
     // Non-paired instruction.
     MIB = BuildMI(*I->getParent(), I, I->getDebugLoc(), TII->get(NewOpc))
               .add(getLdStRegOp(*Update))
               .add(getLdStRegOp(*I))
-              .add(getLdStBaseOp(*I))
+              .add(AArch64InstrInfo::getLdStBaseOp(*I))
               .addImm(Value / Scale)
               .setMemRefs(I->memoperands())
               .setMIFlags(I->mergeFlagsWith(*Update));
   } else {
     // Paired instruction.
     MIB = BuildMI(*I->getParent(), I, I->getDebugLoc(), TII->get(NewOpc))
               .add(getLdStRegOp(*Update))
               .add(getLdStRegOp(*I, 0))
               .add(getLdStRegOp(*I, 1))
-              .add(getLdStBaseOp(*I))
+              .add(AArch64InstrInfo::getLdStBaseOp(*I))
               .addImm(Value / Scale)
               .setMemRefs(I->memoperands())
               .setMIFlags(I->mergeFlagsWith(*Update));
   }
   if (CFI != E) {
     MachineBasicBlock *MBB = I->getParent();
     MBB->splice(std::next(MIB.getInstr()->getIterator()), MBB, CFI);
   }

 }
 
 MachineBasicBlock::iterator AArch64LoadStoreOpt::findMatchingUpdateInsnForward(
     MachineBasicBlock::iterator I, int UnscaledOffset, unsigned Limit) {
   MachineBasicBlock::iterator E = I->getParent()->end();
   MachineInstr &MemMI = *I;
   MachineBasicBlock::iterator MBBI = I;
 
-  Register BaseReg = getLdStBaseOp(MemMI).getReg();
-  int MIUnscaledOffset = getLdStOffsetOp(MemMI).getImm() * TII->getMemScale(MemMI);
+  Register BaseReg = AArch64InstrInfo::getLdStBaseOp(MemMI).getReg();
+  int MIUnscaledOffset = AArch64InstrInfo::getLdStOffsetOp(MemMI).getImm() *
+                         TII->getMemScale(MemMI);
 
   // Scan forward looking for post-index opportunities.  Updating instructions
   // can't be formed if the memory instruction doesn't have the offset we're
   // looking for.
   if (MIUnscaledOffset != UnscaledOffset)
     return E;
 
   // If the base register overlaps a source/destination register, we can't
   // merge the update. This does not apply to tag store instructions which
   // ignore the address part of the source register.
   // This does not apply to STGPi as well, which does not have unpredictable
   // behavior in this case unlike normal stores, and always performs writeback
   // after reading the source register value.
   if (!isTagStore(MemMI) && MemMI.getOpcode() != AArch64::STGPi) {
-    bool IsPairedInsn = isPairedLdSt(MemMI);
+    bool IsPairedInsn = AArch64InstrInfo::isPairedLdSt(MemMI);
     for (unsigned i = 0, e = IsPairedInsn ? 2 : 1; i != e; ++i) {
       Register DestReg = getLdStRegOp(MemMI, i).getReg();
       if (DestReg == BaseReg || TRI->isSubRegister(BaseReg, DestReg))
         return E;
     }
   }
 
   // Track which register units have been modified and used between the first

 MachineBasicBlock::iterator AArch64LoadStoreOpt::findMatchingUpdateInsnBackward(
     MachineBasicBlock::iterator I, unsigned Limit) {
   MachineBasicBlock::iterator B = I->getParent()->begin();
   MachineBasicBlock::iterator E = I->getParent()->end();
   MachineInstr &MemMI = *I;
   MachineBasicBlock::iterator MBBI = I;
   MachineFunction &MF = *MemMI.getMF();
 
-  Register BaseReg = getLdStBaseOp(MemMI).getReg();
-  int Offset = getLdStOffsetOp(MemMI).getImm();
+  Register BaseReg = AArch64InstrInfo::getLdStBaseOp(MemMI).getReg();
+  int Offset = AArch64InstrInfo::getLdStOffsetOp(MemMI).getImm();
 
   // If the load/store is the first instruction in the block, there's obviously
   // not any matching update. Ditto if the memory offset isn't zero.
   if (MBBI == B || Offset != 0)
     return E;
   // If the base register overlaps a destination register, we can't
   // merge the update.
   if (!isTagStore(MemMI)) {
-    bool IsPairedInsn = isPairedLdSt(MemMI);
+    bool IsPairedInsn = AArch64InstrInfo::isPairedLdSt(MemMI);
     for (unsigned i = 0, e = IsPairedInsn ? 2 : 1; i != e; ++i) {
       Register DestReg = getLdStRegOp(MemMI, i).getReg();
       if (DestReg == BaseReg || TRI->isSubRegister(BaseReg, DestReg))
         return E;
     }
   }
 
   const bool BaseRegSP = BaseReg == AArch64::SP;
bool AArch64LoadStoreOpt::tryToPromoteLoadFromStore(
     MachineBasicBlock::iterator &MBBI) {
   MachineInstr &MI = *MBBI;
   // If this is a volatile load, don't mess with it.
   if (MI.hasOrderedMemoryRef())
     return false;
 
   // Make sure this is a reg+imm.
   // FIXME: It is possible to extend it to handle reg+reg cases.
-  if (!getLdStOffsetOp(MI).isImm())
+  if (!AArch64InstrInfo::getLdStOffsetOp(MI).isImm())
     return false;
 
   // Look backward up to LdStLimit instructions.
   MachineBasicBlock::iterator StoreI;
   if (findMatchingStore(MBBI, LdStLimit, StoreI)) {
     ++NumLoadsFromStoresPromoted;
     // Promote the load. Keeping the iterator straight is a
     // pain, so we let the merge routine tell us what the next instruction
bool AArch64LoadStoreOpt::tryToPairLdStInst(MachineBasicBlock::iterator &MBBI) {
 
   if (!TII->isCandidateToMergeOrPair(MI))
     return false;
 
   // Early exit if the offset is not possible to match. (6 bits of positive
   // range, plus allow an extra one in case we find a later insn that matches
   // with Offset-1)
   bool IsUnscaled = TII->hasUnscaledLdStOffset(MI);
-  int Offset = getLdStOffsetOp(MI).getImm();
+  int Offset = AArch64InstrInfo::getLdStOffsetOp(MI).getImm();
   int OffsetStride = IsUnscaled ? TII->getMemScale(MI) : 1;
   // Allow one more for offset.
   if (Offset > 0)
     Offset -= OffsetStride;
   if (!inBoundsForPair(IsUnscaled, Offset, OffsetStride))
     return false;
 
   // Look ahead up to LdStLimit instructions for a pairable instruction.
if (Update != E) {
     // Merge the update into the ld/st.
     MBBI = mergeUpdateInsn(MBBI, Update, /*IsPreIdx=*/true);
     return true;
   }
 
   // The immediate in the load/store is scaled by the size of the memory
   // operation. The immediate in the add we're looking for,
   // however, is not, so adjust here.
-  int UnscaledOffset = getLdStOffsetOp(MI).getImm() * TII->getMemScale(MI);
+  int UnscaledOffset =
+      AArch64InstrInfo::getLdStOffsetOp(MI).getImm() * TII->getMemScale(MI);
 
   // Look forward to try to find a pre-index instruction. For example,
   // ldr x1, [x0, #64]
   // add x0, x0, #64
   //   merged into:
   // ldr x1, [x0, #64]!
   Update = findMatchingUpdateInsnForward(MBBI, UnscaledOffset, UpdateLimit);
   if (Update != E) {

llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp

 //===- AArch64MachineScheduler.cpp - MI Scheduler for AArch64 -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AArch64MachineScheduler.h"
+#include "AArch64InstrInfo.h"
+#include "AArch64Subtarget.h"
 #include "MCTargetDesc/AArch64MCTargetDesc.h"
 
 using namespace llvm;
 
+static bool needReorderStoreMI(const MachineInstr *MI) {
+  if (!MI)
+    return false;
+
+  switch (MI->getOpcode()) {
+  default:
+    return false;
+  case AArch64::STURQi:
+  case AArch64::STRQui:
+    if (MI->getMF()->getSubtarget<AArch64Subtarget>().isStoreAddressAscend())
+       return false;
+    LLVM_FALLTHROUGH;
+  case AArch64::STPQi:
+    return AArch64InstrInfo::getLdStOffsetOp(*MI).getType() == MachineOperand::MO_Immediate;
+  }
+
+  return false;
+}
+
+// Return true if two stores with same base address may overlap writes
+static bool mayOverlapWrite(const MachineInstr &MI0, const MachineInstr &MI1,
+                            int64_t &Off0, int64_t &Off1) {
+  const MachineOperand &Base0 = AArch64InstrInfo::getLdStBaseOp(MI0);
+  const MachineOperand &Base1 = AArch64InstrInfo::getLdStBaseOp(MI1);
+
+  // May overlapping writes if two store instructions without same base
+  if (!Base0.isIdenticalTo(Base1))
+    return true;
+
+  int StoreSize0 = AArch64InstrInfo::getMemScale(MI0);
+  int StoreSize1 = AArch64InstrInfo::getMemScale(MI1);
+  Off0 = AArch64InstrInfo::hasUnscaledLdStOffset(MI0.getOpcode())
+             ? AArch64InstrInfo::getLdStOffsetOp(MI0).getImm()
+             : AArch64InstrInfo::getLdStOffsetOp(MI0).getImm() * StoreSize0;
+  Off1 = AArch64InstrInfo::hasUnscaledLdStOffset(MI1.getOpcode())
+             ? AArch64InstrInfo::getLdStOffsetOp(MI1).getImm()
+             : AArch64InstrInfo::getLdStOffsetOp(MI1).getImm() * StoreSize1;
+
+  const MachineInstr &MI = (Off0 < Off1) ? MI0 : MI1;
+  int Multiples = AArch64InstrInfo::isPairedLdSt(MI) ? 2 : 1;
+  int StoreSize = AArch64InstrInfo::getMemScale(MI) * Multiples;
+
+  return llabs(Off0 - Off1) < StoreSize;
+}
+
 bool AArch64PostRASchedStrategy::tryCandidate(SchedCandidate &Cand,
                                               SchedCandidate &TryCand) {
   bool OriginalResult = PostGenericScheduler::tryCandidate(Cand, TryCand);
 
   if (Cand.isValid()) {
     MachineInstr *Instr0 = TryCand.SU->getInstr();
     MachineInstr *Instr1 = Cand.SU->getInstr();
-    // When dealing with two STPqi's.
-    if (Instr0 && Instr1 && Instr0->getOpcode() == Instr1->getOpcode () &&
-        Instr0->getOpcode() == AArch64::STPQi)
-    {
-      MachineOperand &Base0 = Instr0->getOperand(2);
-      MachineOperand &Base1 = Instr1->getOperand(2);
-      int64_t Off0 = Instr0->getOperand(3).getImm();
-      int64_t Off1 = Instr1->getOperand(3).getImm();
-      // With the same base address and non-overlapping writes.
-      if (Base0.isIdenticalTo(Base1) && llabs (Off0 - Off1) >= 2) {
-        TryCand.Reason = NodeOrder;
-        // Order them by ascending offsets.
-        return Off0 < Off1;
-      }
-    }
-  }
+
+    if (!needReorderStoreMI(Instr0) || !needReorderStoreMI(Instr1))
+      return OriginalResult;
+
+    int64_t Off0, Off1;
+    if (!mayOverlapWrite(*Instr0, *Instr1, Off0, Off1)) {
 
   return OriginalResult;
 }