[llvm-commits] [llvm] r134114 - in /llvm/trunk: lib/Target/ARM/ARMAsmPrinter.cpp lib/Target/ARM/ARMBaseRegisterInfo.cpp lib/Target/ARM/ARMInstrThumb2.td lib/Target/ARM/ARMLoadStoreOptimizer.cpp lib/Target/ARM/Thumb2InstrInfo.cpp lib/Target/ARM/Thumb2SizeReduction.cpp utils/TableGen/ARMDecoderEmitter.cpp

Jim Grosbach grosbach at apple.com
Wed Jun 29 19:23:59 PDT 2011


Doh. Fixed in r134131.


-j
On Jun 29, 2011, at 6:49 PM, Evan Cheng wrote:

> Thumb2InstrInfo.cpp:422:10: warning: unused variable 'isSP' [-Wunused-variable]
>    bool isSP = FrameReg == ARM::SP;
>         ^
> 
> On Jun 29, 2011, at 4:25 PM, Jim Grosbach wrote:
> 
>> Author: grosbach
>> Date: Wed Jun 29 18:25:04 2011
>> New Revision: 134114
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=134114&view=rev
>> Log:
>> Remove redundant Thumb2 ADD/SUB SP instruction definitions.
>> 
>> Unlike Thumb1, Thumb2 does not have dedicated encodings for adjusting the
>> stack pointer. It can just use the normal add-register-immediate encoding
>> since it can use all registers as a source, not just R0-R7. The extra
>> instruction definitions are just duplicates of the normal instructions with
>> the (not well enforced) constraint that the source register was SP.
>> 
>> 
>> Modified:
>>   llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
>>   llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp
>>   llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td
>>   llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
>>   llvm/trunk/lib/Target/ARM/Thumb2InstrInfo.cpp
>>   llvm/trunk/lib/Target/ARM/Thumb2SizeReduction.cpp
>>   llvm/trunk/utils/TableGen/ARMDecoderEmitter.cpp
>> 
>> Modified: llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp?rev=134114&r1=134113&r2=134114&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp Wed Jun 29 18:25:04 2011
>> @@ -1018,11 +1018,10 @@
>>        Offset = -MI->getOperand(2).getImm();
>>        break;
>>      case ARM::SUBri:
>> -      case ARM::t2SUBrSPi:
>> -        Offset =  MI->getOperand(2).getImm();
>> +        Offset = MI->getOperand(2).getImm();
>>        break;
>>      case ARM::tSUBspi:
>> -        Offset =  MI->getOperand(2).getImm()*4;
>> +        Offset = MI->getOperand(2).getImm()*4;
>>        break;
>>      case ARM::tADDspi:
>>      case ARM::tADDrSPi:
>> @@ -1097,13 +1096,6 @@
>>    OutStreamer.EmitInstruction(TmpInst);
>>    return;
>>  }
>> -  case ARM::t2ADDrSPi:
>> -  case ARM::t2ADDrSPi12:
>> -  case ARM::t2SUBrSPi:
>> -  case ARM::t2SUBrSPi12:
>> -    assert ((MI->getOperand(1).getReg() == ARM::SP) &&
>> -            "Unexpected source register!");
>> -    break;
>> 
>>  case ARM::t2MOVi32imm: assert(0 && "Should be lowered by thumb2it pass");
>>  case ARM::DBG_VALUE: {
>> 
>> Modified: llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp?rev=134114&r1=134113&r2=134114&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp Wed Jun 29 18:25:04 2011
>> @@ -1284,9 +1284,5 @@
>>    }
>>    // Update the original instruction to use the scratch register.
>>    MI.getOperand(i).ChangeToRegister(ScratchReg, false, false, true);
>> -    if (MI.getOpcode() == ARM::t2ADDrSPi)
>> -      MI.setDesc(TII.get(ARM::t2ADDri));
>> -    else if (MI.getOpcode() == ARM::t2SUBrSPi)
>> -      MI.setDesc(TII.get(ARM::t2SUBri));
>>  }
>> }
>> 
>> Modified: llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td?rev=134114&r1=134113&r2=134114&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td Wed Jun 29 18:25:04 2011
>> @@ -1169,63 +1169,6 @@
>>                                []>;
>> 
>> 
>> -// FIXME: None of these add/sub SP special instructions should be necessary
>> -// at all for thumb2 since they use the same encodings as the generic
>> -// add/sub instructions. In thumb1 we need them since they have dedicated
>> -// encodings. At the least, they should be pseudo instructions.
>> -// ADD r, sp, {so_imm|i12}
>> -let isCodeGenOnly = 1 in {
>> -def t2ADDrSPi   : T2sTwoRegImm<(outs GPR:$Rd), (ins GPR:$Rn, t2_so_imm:$imm),
>> -                        IIC_iALUi, "add", ".w\t$Rd, $Rn, $imm", []> {
>> -  let Inst{31-27} = 0b11110;
>> -  let Inst{25} = 0;
>> -  let Inst{24-21} = 0b1000;
>> -  let Inst{15} = 0;
>> -}
>> -def t2ADDrSPi12 : T2TwoRegImm<(outs GPR:$Rd), (ins GPR:$Rn, imm0_4095:$imm),
>> -                       IIC_iALUi, "addw", "\t$Rd, $Rn, $imm", []> {
>> -  let Inst{31-27} = 0b11110;
>> -  let Inst{25-20} = 0b100000;
>> -  let Inst{15} = 0;
>> -}
>> -
>> -// ADD r, sp, so_reg
>> -def t2ADDrSPs   : T2sTwoRegShiftedReg<
>> -                        (outs GPR:$Rd), (ins GPR:$Rn, t2_so_reg:$ShiftedRm),
>> -                        IIC_iALUsi, "add", ".w\t$Rd, $Rn, $ShiftedRm", []> {
>> -  let Inst{31-27} = 0b11101;
>> -  let Inst{26-25} = 0b01;
>> -  let Inst{24-21} = 0b1000;
>> -  let Inst{15} = 0;
>> -}
>> -
>> -// SUB r, sp, {so_imm|i12}
>> -def t2SUBrSPi   : T2sTwoRegImm<(outs GPR:$Rd), (ins GPR:$Rn, t2_so_imm:$imm),
>> -                        IIC_iALUi, "sub", ".w\t$Rd, $Rn, $imm", []> {
>> -  let Inst{31-27} = 0b11110;
>> -  let Inst{25} = 0;
>> -  let Inst{24-21} = 0b1101;
>> -  let Inst{15} = 0;
>> -}
>> -def t2SUBrSPi12 : T2TwoRegImm<(outs GPR:$Rd), (ins GPR:$Rn, imm0_4095:$imm),
>> -                       IIC_iALUi, "subw", "\t$Rd, $Rn, $imm", []> {
>> -  let Inst{31-27} = 0b11110;
>> -  let Inst{25-20} = 0b101010;
>> -  let Inst{15} = 0;
>> -}
>> -
>> -// SUB r, sp, so_reg
>> -def t2SUBrSPs   : T2sTwoRegImm<(outs GPR:$Rd), (ins GPR:$Rn, t2_so_reg:$imm),
>> -                       IIC_iALUsi,
>> -                       "sub", "\t$Rd, $Rn, $imm", []> {
>> -  let Inst{31-27} = 0b11101;
>> -  let Inst{26-25} = 0b01;
>> -  let Inst{24-21} = 0b1101;
>> -  let Inst{19-16} = 0b1101; // Rn = sp
>> -  let Inst{15} = 0;
>> -}
>> -} // end isCodeGenOnly = 1
>> -
>> //===----------------------------------------------------------------------===//
>> //  Load / store Instructions.
>> //
>> 
>> Modified: llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp?rev=134114&r1=134113&r2=134114&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp Wed Jun 29 18:25:04 2011
>> @@ -329,13 +329,9 @@
>>      if (NewBase == 0)
>>        return false;
>>    }
>> -    int BaseOpc = !isThumb2
>> -      ? ARM::ADDri
>> -      : ((Base == ARM::SP) ? ARM::t2ADDrSPi : ARM::t2ADDri);
>> +    int BaseOpc = !isThumb2 ? ARM::ADDri : ARM::t2ADDri;
>>    if (Offset < 0) {
>> -      BaseOpc = !isThumb2
>> -        ? ARM::SUBri
>> -        : ((Base == ARM::SP) ? ARM::t2SUBrSPi : ARM::t2SUBri);
>> +      BaseOpc = !isThumb2 ? ARM::SUBri : ARM::t2SUBri;
>>      Offset = - Offset;
>>    }
>>    int ImmedOffset = isThumb2
>> @@ -516,8 +512,6 @@
>>  if (!MI)
>>    return false;
>>  if (MI->getOpcode() != ARM::t2SUBri &&
>> -      MI->getOpcode() != ARM::t2SUBrSPi &&
>> -      MI->getOpcode() != ARM::t2SUBrSPi12 &&
>>      MI->getOpcode() != ARM::tSUBspi &&
>>      MI->getOpcode() != ARM::SUBri)
>>    return false;
>> @@ -541,8 +535,6 @@
>>  if (!MI)
>>    return false;
>>  if (MI->getOpcode() != ARM::t2ADDri &&
>> -      MI->getOpcode() != ARM::t2ADDrSPi &&
>> -      MI->getOpcode() != ARM::t2ADDrSPi12 &&
>>      MI->getOpcode() != ARM::tADDspi &&
>>      MI->getOpcode() != ARM::ADDri)
>>    return false;
>> 
>> Modified: llvm/trunk/lib/Target/ARM/Thumb2InstrInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Thumb2InstrInfo.cpp?rev=134114&r1=134113&r2=134114&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/Thumb2InstrInfo.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/Thumb2InstrInfo.cpp Wed Jun 29 18:25:04 2011
>> @@ -251,7 +251,7 @@
>>      }
>> 
>>      // sub rd, sp, so_imm
>> -      Opc = isSub ? ARM::t2SUBrSPi : ARM::t2ADDrSPi;
>> +      Opc = isSub ? ARM::t2SUBri : ARM::t2ADDri;
>>      if (ARM_AM::getT2SOImmVal(NumBytes) != -1) {
>>        NumBytes = 0;
>>      } else {
>> @@ -425,9 +425,9 @@
>>    if (Offset < 0) {
>>      Offset = -Offset;
>>      isSub = true;
>> -      MI.setDesc(TII.get(isSP ? ARM::t2SUBrSPi : ARM::t2SUBri));
>> +      MI.setDesc(TII.get(ARM::t2SUBri));
>>    } else {
>> -      MI.setDesc(TII.get(isSP ? ARM::t2ADDrSPi : ARM::t2ADDri));
>> +      MI.setDesc(TII.get(ARM::t2ADDri));
>>    }
>> 
>>    // Common case: small offset, fits into instruction.
>> @@ -443,9 +443,7 @@
>>    // Another common case: imm12.
>>    if (Offset < 4096 &&
>>        (!HasCCOut || MI.getOperand(MI.getNumOperands()-1).getReg() == 0)) {
>> -      unsigned NewOpc = isSP
>> -        ? (isSub ? ARM::t2SUBrSPi12 : ARM::t2ADDrSPi12)
>> -        : (isSub ? ARM::t2SUBri12   : ARM::t2ADDri12);
>> +      unsigned NewOpc = isSub ? ARM::t2SUBri12 : ARM::t2ADDri12;
>>      MI.setDesc(TII.get(NewOpc));
>>      MI.getOperand(FrameRegIdx).ChangeToRegister(FrameReg, false);
>>      MI.getOperand(FrameRegIdx+1).ChangeToImmediate(Offset);
>> 
>> Modified: llvm/trunk/lib/Target/ARM/Thumb2SizeReduction.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Thumb2SizeReduction.cpp?rev=134114&r1=134113&r2=134114&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/Thumb2SizeReduction.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/Thumb2SizeReduction.cpp Wed Jun 29 18:25:04 2011
>> @@ -57,10 +57,8 @@
>>  static const ReduceEntry ReduceTable[] = {
>>    // Wide,        Narrow1,      Narrow2,     imm1,imm2,  lo1, lo2, P/C, PF, S
>>    { ARM::t2ADCrr, 0,            ARM::tADC,     0,   0,    0,   1,  0,0, 0,0 },
>> -    { ARM::t2ADDri, ARM::tADDi3,  ARM::tADDi8,   3,   8,    1,   1,  0,0, 0,0 },
>> +    { ARM::t2ADDri, ARM::tADDi3,  ARM::tADDi8,   3,   8,    1,   1,  0,0, 0,1 },
>>    { ARM::t2ADDrr, ARM::tADDrr,  ARM::tADDhirr, 0,   0,    1,   0,  0,1, 0,0 },
>> -    // Note: immediate scale is 4.
>> -    { ARM::t2ADDrSPi,ARM::tADDrSPi,0,            8,   0,    1,   0,  1,0, 0,1 },
>>    { ARM::t2ADDSri,ARM::tADDi3,  ARM::tADDi8,   3,   8,    1,   1,  2,2, 0,1 },
>>    { ARM::t2ADDSrr,ARM::tADDrr,  0,             0,   0,    1,   0,  2,0, 0,1 },
>>    { ARM::t2ANDrr, 0,            ARM::tAND,     0,   0,    0,   1,  0,0, 1,0 },
>> @@ -291,7 +289,7 @@
>>                 Opc == ARM::t2LDMDB     || Opc == ARM::t2LDMIA_UPD ||
>>                 Opc == ARM::t2LDMDB_UPD);
>>  bool isLROk = (Opc == ARM::t2STMIA_UPD || Opc == ARM::t2STMDB_UPD);
>> -  bool isSPOk = isPCOk || isLROk || (Opc == ARM::t2ADDrSPi);
>> +  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())
>> @@ -481,6 +479,44 @@
>> Thumb2SizeReduce::ReduceSpecial(MachineBasicBlock &MBB, MachineInstr *MI,
>>                                const ReduceEntry &Entry,
>>                                bool LiveCPSR, MachineInstr *CPSRDef) {
>> +  unsigned Opc = MI->getOpcode();
>> +  if (Opc == ARM::t2ADDri) {
>> +    // If the source register is SP, try to reduce to tADDrSPi, otherwise
>> +    // it's a normal reduce.
>> +    if (MI->getOperand(1).getReg() != ARM::SP) {
>> +      if (ReduceTo2Addr(MBB, MI, Entry, LiveCPSR, CPSRDef))
>> +        return true;
>> +      return ReduceToNarrow(MBB, MI, Entry, LiveCPSR, CPSRDef);
>> +    }
>> +    // Try to reduce to tADDrSPi.
>> +    unsigned Imm = MI->getOperand(2).getImm();
>> +    // The immediate must be in range, the destination register must be a low
>> +    // reg, and the condition flags must not be being set.
>> +    if (Imm & 3 || Imm > 1024)
>> +      return false;
>> +    if (!isARMLowRegister(MI->getOperand(0).getReg()))
>> +      return false;
>> +    const MCInstrDesc &MCID = MI->getDesc();
>> +    if (MCID.hasOptionalDef() &&
>> +        MI->getOperand(MCID.getNumOperands()-1).getReg() == ARM::CPSR)
>> +      return false;
>> +
>> +    MachineInstrBuilder MIB = BuildMI(MBB, *MI, MI->getDebugLoc(),
>> +                                      TII->get(ARM::tADDrSPi))
>> +      .addOperand(MI->getOperand(0))
>> +      .addOperand(MI->getOperand(1))
>> +      .addImm(Imm / 4); // The tADDrSPi has an implied scale by four.
>> +
>> +    // Transfer MI flags.
>> +    MIB.setMIFlags(MI->getFlags());
>> +
>> +    DEBUG(errs() << "Converted 32-bit: " << *MI << "       to 16-bit: " <<*MIB);
>> +
>> +    MBB.erase(MI);
>> +    ++NumNarrows;
>> +    return true;
>> +  }
>> +
>>  if (Entry.LowRegs1 && !VerifyLowRegs(MI))
>>    return false;
>> 
>> @@ -488,7 +524,6 @@
>>  if (MCID.mayLoad() || MCID.mayStore())
>>    return ReduceLoadStore(MBB, MI, Entry);
>> 
>> -  unsigned Opc = MI->getOpcode();
>>  switch (Opc) {
>>  default: break;
>>  case ARM::t2ADDSri:
>> @@ -531,13 +566,6 @@
>>      return true;
>>    return ReduceToNarrow(MBB, MI, Entry, LiveCPSR, CPSRDef);
>>  }
>> -  case ARM::t2ADDrSPi: {
>> -    static const ReduceEntry NarrowEntry =
>> -      { ARM::t2ADDrSPi,ARM::tADDspi, 0, 7, 0, 1, 0, 1, 0, 0,1 };
>> -    if (MI->getOperand(0).getReg() == ARM::SP)
>> -      return ReduceToNarrow(MBB, MI, NarrowEntry, LiveCPSR, CPSRDef);
>> -    return ReduceToNarrow(MBB, MI, Entry, LiveCPSR, CPSRDef);
>> -  }
>>  }
>>  return false;
>> }
>> @@ -645,9 +673,8 @@
>>    return false;
>> 
>>  unsigned Limit = ~0U;
>> -  unsigned Scale = (Entry.WideOpc == ARM::t2ADDrSPi) ? 4 : 1;
>>  if (Entry.Imm1Limit)
>> -    Limit = ((1 << Entry.Imm1Limit) - 1) * Scale;
>> +    Limit = (1 << Entry.Imm1Limit) - 1;
>> 
>>  const MCInstrDesc &MCID = MI->getDesc();
>>  for (unsigned i = 0, e = MCID.getNumOperands(); i != e; ++i) {
>> @@ -658,13 +685,11 @@
>>      unsigned Reg = MO.getReg();
>>      if (!Reg || Reg == ARM::CPSR)
>>        continue;
>> -      if (Entry.WideOpc == ARM::t2ADDrSPi && Reg == ARM::SP)
>> -        continue;
>>      if (Entry.LowRegs1 && !isARMLowRegister(Reg))
>>        return false;
>>    } else if (MO.isImm() &&
>>               !MCID.OpInfo[i].isPredicate()) {
>> -      if (((unsigned)MO.getImm()) > Limit || (MO.getImm() & (Scale-1)) != 0)
>> +      if (((unsigned)MO.getImm()) > Limit)
>>        return false;
>>    }
>>  }
>> @@ -723,15 +748,11 @@
>>    if (SkipPred && isPred)
>>        continue;
>>    const MachineOperand &MO = MI->getOperand(i);
>> -    if (Scale > 1 && !isPred && MO.isImm())
>> -      MIB.addImm(MO.getImm() / Scale);
>> -    else {
>> -      if (MO.isReg() && MO.isImplicit() && MO.getReg() == ARM::CPSR)
>> -        // Skip implicit def of CPSR. Either it's modeled as an optional
>> -        // def now or it's already an implicit def on the new instruction.
>> -        continue;
>> -      MIB.addOperand(MO);
>> -    }
>> +    if (MO.isReg() && MO.isImplicit() && MO.getReg() == ARM::CPSR)
>> +      // Skip implicit def of CPSR. Either it's modeled as an optional
>> +      // def now or it's already an implicit def on the new instruction.
>> +      continue;
>> +    MIB.addOperand(MO);
>>  }
>>  if (!MCID.isPredicable() && NewMCID.isPredicable())
>>    AddDefaultPred(MIB);
>> 
>> Modified: llvm/trunk/utils/TableGen/ARMDecoderEmitter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/ARMDecoderEmitter.cpp?rev=134114&r1=134113&r2=134114&view=diff
>> ==============================================================================
>> --- llvm/trunk/utils/TableGen/ARMDecoderEmitter.cpp (original)
>> +++ llvm/trunk/utils/TableGen/ARMDecoderEmitter.cpp Wed Jun 29 18:25:04 2011
>> @@ -1640,12 +1640,8 @@
>>    // Ignore tADDrSP, tADDspr, and tPICADD, prefer the generic tADDhirr.
>>    // Ignore t2SUBrSPs, prefer the t2SUB[S]r[r|s].
>>    // Ignore t2ADDrSPs, prefer the t2ADD[S]r[r|s].
>> -    // Ignore t2ADDrSPi/t2SUBrSPi, which have more generic couterparts.
>> -    // Ignore t2ADDrSPi12/t2SUBrSPi12, which have more generic couterparts
>>    if (Name == "tADDrSP" || Name == "tADDspr" || Name == "tPICADD" ||
>> -        Name == "t2SUBrSPs" || Name == "t2ADDrSPs" ||
>> -        Name == "t2ADDrSPi" || Name == "t2SUBrSPi" ||
>> -        Name == "t2ADDrSPi12" || Name == "t2SUBrSPi12")
>> +        Name == "t2SUBrSPs" || Name == "t2ADDrSPs")
>>      return false;
>> 
>>    // FIXME: Use ldr.n to work around a Darwin assembler bug.
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 




More information about the llvm-commits mailing list