[AArch64] AArch64LoadStoreOptimizer: fix bug in pre-inc check iterator

[gberry]

Summary

Fix case where a pre-inc/dec load/store would not be formed if the
add/sub that forms the inc/dec part of the operation was the first
instruction in the block being examined.

[mcrosier]

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

957 ↗

(On Diff #46572)

Did you mean to delete lines 954 and 955? Otherwise, I'm not sure why this check would be necessary.

1454 ↗

(On Diff #46572)

Space between the while and the (.

Or why not just while (MBBI != B)?

[junbuml]

Looks good to me assuming Chad's comment fixed.

[gberry]

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

957 ↗

(On Diff #46572)

No, lines 954, 955 filter out non-register operands, this new check filters out register operands of register 0, which is not a real register. These can show up in dbg_value instructions, which I removed as a special case in the loop below.
This idiom (check for reg operand, then check for reg != 0) appears in quite a few places.

1454 ↗

(On Diff #46572)

Yep and yep.

[mcrosier]

LGTM once you've addressed the minor nits.

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

957 ↗

(On Diff #46572)

Ah, the subtle difference between !isReg and !getReg.. Thanks for the clarification.

[gberry]

Address Chad's comments.

[mcrosier]

This may need a trivial fix after r259828.

[gberry]

Rebased after r259828

llvm/trunk/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

static void trackRegDefsUses(const MachineInstr *MI, BitVector &ModifiedRegs,
                              const TargetRegisterInfo *TRI) {
   for (const MachineOperand &MO : MI->operands()) {
     if (MO.isRegMask())
       ModifiedRegs.setBitsNotInMask(MO.getRegMask());
 
     if (!MO.isReg())
       continue;
     unsigned Reg = MO.getReg();
+    if (!Reg)
+      continue;
     if (MO.isDef()) {
       for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI)
         ModifiedRegs.set(*AI);
     } else {
       assert(MO.isUse() && "Reg operand not a def and not a use?!?");
       for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI)
         UsedRegs.set(*AI);
     }
for (unsigned i = 0, e = IsPairedInsn ? 2 : 1; i != e; ++i) {
     if (DestReg == BaseReg || TRI->isSubRegister(BaseReg, DestReg))
       return E;
   }
 
   // Track which registers have been modified and used between the first insn
   // (inclusive) and the second insn.
   ModifiedRegs.reset();
   UsedRegs.reset();
+  unsigned Count = 0;
+  do {
     --MBBI;
-  for (unsigned Count = 0; MBBI != B && Count < Limit; --MBBI) {
     MachineInstr *MI = MBBI;
-    // Skip DBG_VALUE instructions.
-    if (MI->isDebugValue())
-      continue;
 
-    // Now that we know this is a real instruction, count it.
+    // Don't count DBG_VALUE instructions towards the search limit.
+    if (!MI->isDebugValue())
       ++Count;
 
     // If we found a match, return it.
     if (isMatchingUpdateInsn(I, MI, BaseReg, Offset))
       return MBBI;
 
     // Update the status of what the instruction clobbered and used.
     trackRegDefsUses(MI, ModifiedRegs, UsedRegs, TRI);
 
     // Otherwise, if the base register is used or modified, we have no match, so
     // return early.
     if (ModifiedRegs[BaseReg] || UsedRegs[BaseReg])
       return E;
-  }
+  } while (MBBI != B && Count < Limit);
   return E;
 }
 
 bool AArch64LoadStoreOpt::tryToPromoteLoadFromStore(
     MachineBasicBlock::iterator &MBBI) {
   MachineInstr *MI = MBBI;
   // If this is a volatile load, don't mess with it.
   if (MI->hasOrderedMemoryRef())

llvm/trunk/test/CodeGen/AArch64/arm64-nvcast.ll

 ; RUN: llc < %s -mtriple=arm64-apple-ios | FileCheck %s
 
 ; CHECK-LABEL: _test:
 ; CHECK:  fmov.2d v0, #2.00000000
-; CHECK:  str  q0, [sp]
+; CHECK:  str  q0, [sp, #-16]!
 ; CHECK:  mov  x8, sp
 ; CHECK:  ldr s0, [x8, w1, sxtw #2]
 ; CHECK:  str  s0, [x0]
 
 define void @test(float * %p1, i32 %v1) {
 entry:
   %v2 = extractelement <3 x float> <float 0.000000e+00, float 2.000000e+00, float 0.000000e+00>, i32 %v1
   store float %v2, float* %p1, align 4
   ret void
 }
 
 ; CHECK-LABEL: _test2
 ; CHECK: movi.16b  v0, #0x3f
-; CHECK: str  q0, [sp]
+; CHECK: str  q0, [sp, #-16]!
 ; CHECK: mov  x8, sp
 ; CHECK: ldr s0, [x8, w1, sxtw #2]
 ; CHECK: str  s0, [x0]
 
 define void @test2(float * %p1, i32 %v1) {
 entry:
   %v2 = extractelement <3 x float> <float 0.7470588088035583, float 0.7470588088035583, float 0.7470588088035583>, i32 %v1
   store float %v2, float* %p1, align 4
   ret void
 }