[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

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

1454

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

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

Yep and yep.

[mcrosier]

LGTM once you've addressed the minor nits.

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

957

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

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);
     }
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.
   BitVector ModifiedRegs, UsedRegs;
   ModifiedRegs.resize(TRI->getNumRegs());
   UsedRegs.resize(TRI->getNumRegs());
+  do {
     --MBBI;
-  for (; MBBI != B; --MBBI) {
     MachineInstr *MI = MBBI;
-    // Skip DBG_VALUE instructions.
-    if (MI->isDebugValue())
-      continue;
 
     // 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);
   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())
     return false;
 
   // Make sure this is a reg+imm.

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
 }