[AArch64] Refactoring aarch64-ldst-opt. NCF

[junbuml]

Summary

Summary :

• Rename isSmallTypeLdMerge() to isNarrowLoad().
• Rename NumSmallTypeMerged to NumNarrowTypePromoted.
• Use Subtarget defined as a member variable.

[junbuml]

This is minor refactorings mentioned in D14183.

[mcrosier]

LGTM with a few nits.

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

44

Currently, this is only applicable to loads, correct? I'd be in favor of

STATISTIC(NumNarrowLoadsPromoted, "Number of narrow loads promoted");

When your next patch for zero stores lands you can add an equivalent stat for stores.

1230

NumNarrowTypePromoted -> NumNarrowLoadsPromoted

1470

Shouldn't this be a logical and (i.e., &&)?

No need for the extra parens around !Subtarget->requiresStrictAlign().

Specifically,

return ProfitableArch && !Subtarget->requiresStrictAlign();

[junbuml]

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

44

OK. I will isolate this only for load.

1470

You are right ! Thanks for catching this. I will fix it.

[junbuml]

Addressed Chad's comments

[junbuml]

committed in r253587.

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

 /// AArch64AllocLoadStoreOpt - Post-register allocation pass to combine
 /// load / store instructions to form ldp / stp instructions.
 
 STATISTIC(NumPairCreated, "Number of load/store pair instructions generated");
 STATISTIC(NumPostFolded, "Number of post-index updates folded");
 STATISTIC(NumPreFolded, "Number of pre-index updates folded");
 STATISTIC(NumUnscaledPairCreated,
           "Number of load/store from unscaled generated");
-STATISTIC(NumSmallTypeMerged, "Number of small type loads merged");
+STATISTIC(NumNarrowLoadsPromoted, "Number of narrow loads promoted");
 
 static cl::opt<unsigned> ScanLimit("aarch64-load-store-scan-limit",
                                    cl::init(20), cl::Hidden);
 
 namespace llvm {
 void initializeAArch64LoadStoreOptPass(PassRegistry &);
 }
 
static unsigned getBitExtrOpcode(MachineInstr *MI) {
   case AArch64::LDRSBWui:
   case AArch64::LDURSBWi:
   case AArch64::LDRSHWui:
   case AArch64::LDURSHWi:
     return AArch64::SBFMWri;
   }
 }
 
-static bool isSmallTypeLdMerge(unsigned Opc) {
+static bool isNarrowLoad(unsigned Opc) {
   switch (Opc) {
   default:
     return false;
   case AArch64::LDRHHui:
   case AArch64::LDURHHi:
   case AArch64::LDRBBui:
   case AArch64::LDURBBi:
   case AArch64::LDRSHWui:
   case AArch64::LDURSHWi:
   case AArch64::LDRSBWui:
   case AArch64::LDURSBWi:
     return true;
   }
 }
 
-static bool isSmallTypeLdMerge(MachineInstr *MI) {
-  return isSmallTypeLdMerge(MI->getOpcode());
+static bool isNarrowLoad(MachineInstr *MI) {
+  return isNarrowLoad(MI->getOpcode());
 }
 
 // Scaling factor for unscaled load or store.
 static int getMemScale(MachineInstr *MI) {
   switch (MI->getOpcode()) {
   default:
     llvm_unreachable("Opcode has unknown scale!");
   case AArch64::LDRBBui:
if (SExtIdx != -1)
       SExtIdx = (SExtIdx + 1) % 2;
   } else {
     RtMI = I;
     Rt2MI = Paired;
   }
 
   int OffsetImm = getLdStOffsetOp(RtMI).getImm();
 
-  if (isSmallTypeLdMerge(Opc)) {
+  if (isNarrowLoad(Opc)) {
     // Change the scaled offset from small to large type.
     if (!IsUnscaled) {
       assert(((OffsetImm & 1) == 0) && "Unexpected offset to merge");
       OffsetImm /= 2;
     }
     MachineInstr *RtNewDest = MergeForward ? I : Paired;
     // When merging small (< 32 bit) loads for big-endian targets, the order of
     // the component parts gets swapped.
AArch64LoadStoreOpt::findMatchingInsn(MachineBasicBlock::iterator I,
   // e.g., ldr x0, [x0]
   if (FirstMI->modifiesRegister(BaseReg, TRI))
     return E;
 
   // Early exit if the offset if 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)
   int OffsetStride = IsUnscaled ? getMemScale(FirstMI) : 1;
-  if (!isSmallTypeLdMerge(Opc) &&
-      !inBoundsForPair(IsUnscaled, Offset, OffsetStride))
+  if (!isNarrowLoad(Opc) && !inBoundsForPair(IsUnscaled, Offset, OffsetStride))
     return E;
 
   // Track which registers have been modified and used between the first insn
   // (inclusive) and the second insn.
   BitVector ModifiedRegs, UsedRegs;
   ModifiedRegs.resize(TRI->getNumRegs());
   UsedRegs.resize(TRI->getNumRegs());
 
if (CanMergeOpc && getLdStOffsetOp(MI).isImm()) {
         // If this is a volatile load/store that otherwise matched, stop looking
         // as something is going on that we don't have enough information to
         // safely transform. Similarly, stop if we see a hint to avoid pairs.
         if (MI->hasOrderedMemoryRef() || TII->isLdStPairSuppressed(MI))
           return E;
         // If the resultant immediate offset of merging these instructions
         // is out of range for a pairwise instruction, bail and keep looking.
         bool MIIsUnscaled = isUnscaledLdSt(MI);
-        bool IsSmallTypeLd = isSmallTypeLdMerge(MI->getOpcode());
-        if (!IsSmallTypeLd &&
+        bool IsNarrowLoad = isNarrowLoad(MI->getOpcode());
+        if (!IsNarrowLoad &&
             !inBoundsForPair(MIIsUnscaled, MinOffset, OffsetStride)) {
           trackRegDefsUses(MI, ModifiedRegs, UsedRegs, TRI);
           MemInsns.push_back(MI);
           continue;
         }
 
-        if (IsSmallTypeLd) {
+        if (IsNarrowLoad) {
           // If the alignment requirements of the larger type scaled load
           // instruction can't express the scaled offset of the smaller type
           // input, bail and keep looking.
           if (!IsUnscaled && alignTo(MinOffset, 2) != MinOffset) {
             trackRegDefsUses(MI, ModifiedRegs, UsedRegs, TRI);
             MemInsns.push_back(MI);
             continue;
           }
bool AArch64LoadStoreOpt::tryToMergeLdStInst(
   // MachineMemOperands hints are set by the AArch64StorePairSuppress pass.
   if (TII->isLdStPairSuppressed(MI))
     return false;
 
   // Look ahead up to ScanLimit instructions for a pairable instruction.
   LdStPairFlags Flags;
   MachineBasicBlock::iterator Paired = findMatchingInsn(MBBI, Flags, ScanLimit);
   if (Paired != E) {
-    if (isSmallTypeLdMerge(MI)) {
-      ++NumSmallTypeMerged;
+    if (isNarrowLoad(MI)) {
+      ++NumNarrowLoadsPromoted;
     } else {
       ++NumPairCreated;
       if (isUnscaledLdSt(MI))
         ++NumUnscaledPairCreated;
     }
 
     // Merge the loads into a pair. Keeping the iterator straight is a
     // pain, so we let the merge routine tell us what the next instruction
case AArch64::STPXi: {
       // FIXME: Do the other instructions.
     }
   }
 
   return Modified;
 }
 
 bool AArch64LoadStoreOpt::enableNarrowLdMerge(MachineFunction &Fn) {
-  const AArch64Subtarget *SubTarget =
-      &static_cast<const AArch64Subtarget &>(Fn.getSubtarget());
-  bool ProfitableArch = SubTarget->isCortexA57();
+  bool ProfitableArch = Subtarget->isCortexA57();
   // FIXME: The benefit from converting narrow loads into a wider load could be
   // microarchitectural as it assumes that a single load with two bitfield
   // extracts is cheaper than two narrow loads. Currently, this conversion is
   // enabled only in cortex-a57 on which performance benefits were verified.
-  return ProfitableArch & (!SubTarget->requiresStrictAlign());
+  return ProfitableArch && !Subtarget->requiresStrictAlign();
 }
 
 bool AArch64LoadStoreOpt::runOnMachineFunction(MachineFunction &Fn) {
   Subtarget = &static_cast<const AArch64Subtarget &>(Fn.getSubtarget());
   TII = static_cast<const AArch64InstrInfo *>(Subtarget->getInstrInfo());
   TRI = Subtarget->getRegisterInfo();
 
   bool Modified = false;