[llvm-commits] [llvm] r117858 - in /llvm/trunk: lib/Target/ARM/ARMInstrInfo.td lib/Target/ARM/ARMInstrThumb.td lib/Target/ARM/ARMInstrThumb2.td lib/Target/ARM/ARMInstrVFP.td lib/Target/MBlaze/MBlazeInstrInfo.td utils/TableGen/AsmMatcherEmitter.cpp

Jim Grosbach grosbach at apple.com
Tue Nov 2 18:39:21 PDT 2010


Hi Chris,

I think r118137 has fixed the ARM instructions that were causing heartburn.

-Jim

On Oct 31, 2010, at 11:48 AM, Chris Lattner wrote:

> Author: lattner
> Date: Sun Oct 31 13:48:12 2010
> New Revision: 117858
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=117858&view=rev
> Log:
> the asm matcher can't handle operands with modifiers (like ${foo:bar}).
> Instead of silently ignoring these instructions, emit a hard error and
> force the target author to either refactor the target or mark the 
> instruction 'isCodeGenOnly'.
> 
> Mark a few instructions in ARM and MBlaze as isCodeGenOnly the are 
> doing this.
> 
> Modified:
>    llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
>    llvm/trunk/lib/Target/ARM/ARMInstrThumb.td
>    llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td
>    llvm/trunk/lib/Target/ARM/ARMInstrVFP.td
>    llvm/trunk/lib/Target/MBlaze/MBlazeInstrInfo.td
>    llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrInfo.td?rev=117858&r1=117857&r2=117858&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMInstrInfo.td (original)
> +++ llvm/trunk/lib/Target/ARM/ARMInstrInfo.td Sun Oct 31 13:48:12 2010
> @@ -1166,7 +1166,7 @@
> // FIXME: remove when we have a way to marking a MI with these properties.
> // FIXME: Should pc be an implicit operand like PICADD, etc?
> let isReturn = 1, isTerminator = 1, isBarrier = 1, mayLoad = 1,
> -    hasExtraDefRegAllocReq = 1 in
> +    hasExtraDefRegAllocReq = 1, isCodeGenOnly = 1 in
>   def LDM_RET : AXI4ld<(outs GPR:$wb), (ins addrmode4:$addr, pred:$p,
>                                         reglist:$dsts, variable_ops),
>                        IndexModeUpd, LdStMulFrm, IIC_iLoad_mBr,
> @@ -1416,6 +1416,7 @@
> }
> 
> // Store Return State is a system instruction -- for disassembly only
> +let isCodeGenOnly = 1 in {  // FIXME: This should not use submode!
> def SRSW : ABXI<{1,0,0,?}, (outs), (ins addrmode4:$addr, i32imm:$mode),
>                 NoItinerary, "srs${addr:submode}\tsp!, $mode",
>                 [/* For disassembly only; pattern left blank */]> {
> @@ -1444,6 +1445,7 @@
>   let Inst{31-28} = 0b1111;
>   let Inst{22-20} = 0b001; // W = 0
> }
> +} // isCodeGenOnly = 1
> 
> //===----------------------------------------------------------------------===//
> //  Load / store Instructions.
> @@ -1681,7 +1683,8 @@
> //  Load / store multiple Instructions.
> //
> 
> -let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1 in {
> +let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1,
> +    isCodeGenOnly = 1 in {
> def LDM : AXI4ld<(outs), (ins addrmode4:$addr, pred:$p,
>                           reglist:$dsts, variable_ops),
>                  IndexModeNone, LdStMulFrm, IIC_iLoad_m,
> @@ -1694,7 +1697,8 @@
>                      "$addr.addr = $wb", []>;
> } // mayLoad, neverHasSideEffects, hasExtraDefRegAllocReq
> 
> -let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1 in {
> +let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1,
> +    isCodeGenOnly = 1 in {
> def STM : AXI4st<(outs), (ins addrmode4:$addr, pred:$p,
>                           reglist:$srcs, variable_ops),
>                  IndexModeNone, LdStMulFrm, IIC_iStore_m,
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMInstrThumb.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrThumb.td?rev=117858&r1=117857&r2=117858&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMInstrThumb.td (original)
> +++ llvm/trunk/lib/Target/ARM/ARMInstrThumb.td Sun Oct 31 13:48:12 2010
> @@ -532,7 +532,8 @@
> //
> 
> // These require base address to be written back or one of the loaded regs.
> -let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1 in {
> +let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1,
> +    isCodeGenOnly = 1 in {
> def tLDM : T1I<(outs),
>                (ins addrmode4:$addr, pred:$p, reglist:$dsts, variable_ops),
>                IIC_iLoad_m,
> @@ -547,7 +548,8 @@
>                T1Encoding<{1,1,0,0,1,?}>; // A6.2 & A8.6.53
> } // mayLoad, neverHasSideEffects = 1, hasExtraDefRegAllocReq
> 
> -let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1 in
> +let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1,
> +    isCodeGenOnly = 1 in
> def tSTM_UPD : T1It<(outs tGPR:$wb),
>                     (ins addrmode4:$addr, pred:$p, reglist:$srcs, variable_ops),
>                     IIC_iStore_mu,
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td?rev=117858&r1=117857&r2=117858&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td (original)
> +++ llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td Sun Oct 31 13:48:12 2010
> @@ -1241,7 +1241,8 @@
> //  Load / store multiple Instructions.
> //
> 
> -let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1 in {
> +let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1,
> +    isCodeGenOnly = 1 in {
> def t2LDM : T2XI<(outs), (ins addrmode4:$addr, pred:$p,
>                           reglist:$dsts, variable_ops), IIC_iLoad_m,
>                  "ldm${addr:submode}${p}${addr:wide}\t$addr, $dsts", []> {
> @@ -1267,7 +1268,8 @@
> }
> } // mayLoad, neverHasSideEffects, hasExtraDefRegAllocReq
> 
> -let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1 in {
> +let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1,
> +    isCodeGenOnly = 1 in {
> def t2STM : T2XI<(outs), (ins addrmode4:$addr, pred:$p,
>                           reglist:$srcs, variable_ops), IIC_iStore_m,
>                  "stm${addr:submode}${p}${addr:wide}\t$addr, $srcs", []> {
> @@ -2429,7 +2431,7 @@
> // operand list.
> // FIXME: Should pc be an implicit operand like PICADD, etc?
> let isReturn = 1, isTerminator = 1, isBarrier = 1, mayLoad = 1,
> -    hasExtraDefRegAllocReq = 1 in
> +    hasExtraDefRegAllocReq = 1, isCodeGenOnly = 1 in
>   def t2LDM_RET : T2XIt<(outs GPR:$wb), (ins addrmode4:$addr, pred:$p,
>                                          reglist:$dsts, variable_ops),
>                         IIC_iLoad_mBr,
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMInstrVFP.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrVFP.td?rev=117858&r1=117857&r2=117858&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMInstrVFP.td (original)
> +++ llvm/trunk/lib/Target/ARM/ARMInstrVFP.td Sun Oct 31 13:48:12 2010
> @@ -72,7 +72,8 @@
> //  Load / store multiple Instructions.
> //
> 
> -let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1 in {
> +let mayLoad = 1, neverHasSideEffects = 1, hasExtraDefRegAllocReq = 1,
> +    isCodeGenOnly = 1 in {
> def VLDMD : AXDI4<(outs), (ins addrmode4:$addr, pred:$p, reglist:$dsts,
>                            variable_ops), IndexModeNone, IIC_fpLoad_m,
>                   "vldm${addr:submode}${p}\t$addr, $dsts", "", []> {
> @@ -102,7 +103,8 @@
> }
> } // mayLoad, neverHasSideEffects, hasExtraDefRegAllocReq
> 
> -let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1 in {
> +let mayStore = 1, neverHasSideEffects = 1, hasExtraSrcRegAllocReq = 1,
> +    isCodeGenOnly = 1 in {
> def VSTMD : AXDI4<(outs), (ins addrmode4:$addr, pred:$p, reglist:$srcs,
>                            variable_ops), IndexModeNone, IIC_fpStore_m,
>                   "vstm${addr:submode}${p}\t$addr, $srcs", "", []> {
> 
> Modified: llvm/trunk/lib/Target/MBlaze/MBlazeInstrInfo.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/MBlaze/MBlazeInstrInfo.td?rev=117858&r1=117857&r2=117858&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/MBlaze/MBlazeInstrInfo.td (original)
> +++ llvm/trunk/lib/Target/MBlaze/MBlazeInstrInfo.td Sun Oct 31 13:48:12 2010
> @@ -562,7 +562,7 @@
>                   "src     $dst, $src", [], IIAlu>;
> }
> 
> -let opcode=0x08 in {
> +let opcode=0x08, isCodeGenOnly=1 in {
>   def LEA_ADDI : TB<0x08, (outs GPR:$dst), (ins memri:$addr),
>                     "addi    $dst, ${addr:stackloc}",
>                     [(set GPR:$dst, iaddr:$addr)], IIAlu>;
> 
> Modified: llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp?rev=117858&r1=117857&r2=117858&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp Sun Oct 31 13:48:12 2010
> @@ -257,27 +257,26 @@
>   // this implies a constraint we would not honor.
>   std::set<std::string> OperandNames;
>   for (unsigned i = 1, e = Tokens.size(); i < e; ++i) {
> -    if (Tokens[i][0] == '$' &&
> -        std::find(Tokens[i].begin(),
> -                  Tokens[i].end(), ':') != Tokens[i].end()) {
> -      DEBUG({
> -          errs() << "warning: '" << Name << "': "
> -                 << "ignoring instruction; operand with attribute '"
> -                 << Tokens[i] << "'\n";
> -        });
> -      return false;
> -    }
> -
> -    if (Tokens[i][0] == '$' && !OperandNames.insert(Tokens[i]).second) {
> -      DEBUG({
> +    for (unsigned i = 1, e = Tokens.size(); i < e; ++i) {
> +      if (Tokens[i][0] == '$' &&
> +          Tokens[i].find(':') != StringRef::npos) {
> +        PrintError(CGI.TheDef->getLoc(),
> +                   "instruction with operand modifier '" + Tokens[i].str() +
> +                   "' not supported by asm matcher.  Mark isCodeGenOnly!");
> +        throw std::string("ERROR: Invalid instruction");
> +      }
> +      
> +      if (Tokens[i][0] == '$' && !OperandNames.insert(Tokens[i]).second) {
> +        DEBUG({
>           errs() << "warning: '" << Name << "': "
>                  << "ignoring instruction with tied operand '"
>                  << Tokens[i].str() << "'\n";
>         });
> -      return false;
> +        return false;
> +      }
>     }
>   }
> -
> +  
>   return true;
> }
> 
> @@ -648,13 +647,11 @@
>     case '*': Res += "_STAR_"; break;
>     case '%': Res += "_PCT_"; break;
>     case ':': Res += "_COLON_"; break;
> -
>     default:
> -      if (isalnum(*it))  {
> +      if (isalnum(*it))
>         Res += *it;
> -      } else {
> +      else
>         Res += "_" + utostr((unsigned) *it) + "_";
> -      }
>     }
>   }
> 
> @@ -904,14 +901,6 @@
> }
> 
> void AsmMatcherInfo::BuildInfo(CodeGenTarget &Target) {
> -  // Parse the instructions; we need to do this first so that we can gather the
> -  // singleton register classes.
> -  std::set<std::string> SingletonRegisterNames;
> -
> -  const std::vector<const CodeGenInstruction*> &InstrList =
> -    Target.getInstructionsByEnumValue();
> -  
> -  
>   // Build information about all of the AssemblerPredicates.
>   std::vector<Record*> AllPredicates =
>     Records.getAllDerivedDefinitions("Predicate");
> @@ -931,9 +920,16 @@
>     assert(FeatureNo < 32 && "Too many subtarget features!");
>   }
> 
> +  // Parse the instructions; we need to do this first so that we can gather the
> +  // singleton register classes.
> +  std::set<std::string> SingletonRegisterNames;
> +  const std::vector<const CodeGenInstruction*> &InstrList =
> +    Target.getInstructionsByEnumValue();
>   for (unsigned i = 0, e = InstrList.size(); i != e; ++i) {
>     const CodeGenInstruction &CGI = *InstrList[i];
> 
> +    // If the tblgen -match-prefix option is specified (for tblgen hackers),
> +    // filter the set of instructions we consider.
>     if (!StringRef(CGI.TheDef->getName()).startswith(MatchPrefix))
>       continue;
> 
> @@ -943,7 +939,8 @@
>     II->Instr = &CGI;
>     II->AsmString = FlattenVariants(CGI.AsmString, 0);
> 
> -    // Remove comments from the asm string.
> +    // Remove comments from the asm string.  We know that the asmstring only
> +    // has one line.
>     if (!CommentDelimiter.empty()) {
>       size_t Idx = StringRef(II->AsmString).find(CommentDelimiter);
>       if (Idx != StringRef::npos)
> @@ -955,7 +952,7 @@
>     // Ignore instructions which shouldn't be matched.
>     if (!IsAssemblerInstruction(CGI.TheDef->getName(), CGI, II->Tokens))
>       continue;
> -
> +    
>     // Collect singleton registers, if used.
>     for (unsigned i = 0, e = II->Tokens.size(); i != e; ++i) {
>       if (!II->Tokens[i].startswith(RegisterPrefix))
> 
> 
> _______________________________________________
> 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