Thumb2SizeReduction: Check the correct set of registers for LDMIA.

[pcc]

Summary

The register set for LDMIA begins at offset 3, not 4. We were previously
missing the short encoding of this instruction in the case where the base
register was the first register in the register set.

Also clean up some dead code:

• The isARMLowRegister check is redundant with what VerifyLowRegs does.
• Remove handling of LDMDB instruction, which has no short encoding (and does not appear in ReduceTable).

[rengolin]

Hi Peter,

I can't see a call to VerifyLowRegs in ReduceLoadStore, how can you guarantee that it was indeed checked?

Otherwise, looks ok.

cheers,
--renato

[pcc]

The call is in Thumb2SizeReduce::ReduceSpecial (see line 566 in new code) before the call to ReduceLoadStore.

[rengolin]

In D9485#166124, @pcc wrote:

The call is in Thumb2SizeReduce::ReduceSpecial (see line 566 in new code) before the call to ReduceLoadStore.

Ah, yes, I thought so. What you could do at least is to replace the test into an assert, so that we can catch the odd cases in the future in the case someone decide to call the same function without validating first. And also for documentation. :)

With the assert, LGTM. Thanks!

[pcc]

Ah, yes, I thought so. What you could do at least is to replace the test into an assert, so that we can catch the odd cases in the future in the case someone decide to call the same function without validating first. And also for documentation. :)

Done. I found that the assert was firing in cases where we were loading from SP, so I also needed to remove the t2LDMIA instruction from the set of isPCOk instructions in VerifyLowRegs. This seems fine, as the non-writeback version of this instruction only accepts r0-r7 in the register list.

[pcc]

Add assert

[rengolin]

Perfect, Thanks! LGTM.

lib/Target/ARM/Thumb2SizeReduction.cpp

if (HasCC)
       return false;
   }
 
   return true;
 }
 
 static bool VerifyLowRegs(MachineInstr *MI) {
   unsigned Opc = MI->getOpcode();
-  bool isPCOk = (Opc == ARM::t2LDMIA_RET || Opc == ARM::t2LDMIA     ||
-                 Opc == ARM::t2LDMDB     || Opc == ARM::t2LDMIA_UPD ||
-                 Opc == ARM::t2LDMDB_UPD);
+  bool isPCOk = (Opc == ARM::t2LDMIA_RET || Opc == ARM::t2LDMIA_UPD);
   bool isLROk = (Opc == ARM::t2STMDB_UPD);
   bool isSPOk = isPCOk || isLROk;
   for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
     const MachineOperand &MO = MI->getOperand(i);
     if (!MO.isReg() || MO.isImplicit())
       continue;
     unsigned Reg = MO.getReg();
     if (Reg == 0 || Reg == ARM::CPSR)
Thumb2SizeReduce::ReduceLoadStore(MachineBasicBlock &MBB, MachineInstr *MI,
   case ARM::t2LDRSBs:
   case ARM::t2LDRSHs:
   case ARM::t2STRs:
   case ARM::t2STRBs:
   case ARM::t2STRHs:
     HasShift = true;
     OpNum = 4;
     break;
-  case ARM::t2LDMIA:
-  case ARM::t2LDMDB: {
+  case ARM::t2LDMIA: {
     unsigned BaseReg = MI->getOperand(0).getReg();
-    if (!isARMLowRegister(BaseReg) || Entry.WideOpc != ARM::t2LDMIA)
-      return false;
+    assert(isARMLowRegister(BaseReg));
 
     // For the non-writeback version (this one), the base register must be
     // one of the registers being loaded.
     bool isOK = false;
-    for (unsigned i = 4; i < MI->getNumOperands(); ++i) {
+    for (unsigned i = 3; i < MI->getNumOperands(); ++i) {
       if (MI->getOperand(i).getReg() == BaseReg) {
         isOK = true;
         break;
       }
     }
 
     if (!isOK)
       return false;
 
     OpNum = 0;
     isLdStMul = true;
     break;
   }
   case ARM::t2LDMIA_RET: {
     unsigned BaseReg = MI->getOperand(1).getReg();
     if (BaseReg != ARM::SP)
       return false;
     Opc = Entry.NarrowOpc2; // tPOP_RET
     OpNum = 2;
     isLdStMul = true;
     break;
   }
   case ARM::t2LDMIA_UPD:
-  case ARM::t2LDMDB_UPD:
   case ARM::t2STMIA_UPD:
   case ARM::t2STMDB_UPD: {
     OpNum = 0;
 
     unsigned BaseReg = MI->getOperand(1).getReg();
     if (BaseReg == ARM::SP &&
         (Entry.WideOpc == ARM::t2LDMIA_UPD ||
          Entry.WideOpc == ARM::t2STMDB_UPD)) {

test/CodeGen/Thumb2/float-ops.ll

 ; HARD: vldr s0, [r0]
   %0 = load float, float* %a, align 4
   ret float %0
 }
 
 define double @load_d(double* %a) {
 entry:
 ; CHECK-LABEL: load_d:
-; NONE: ldm.w r0, {r0, r1}
+; NONE: ldm r0, {r0, r1}
 ; HARD: vldr d0, [r0]
   %0 = load double, double* %a, align 8
   ret double %0
 }
 
 define void @store_f(float* %a, float %b) {
 entry:
 ; CHECK-LABEL: store_f: