[llvm] r193185 - ARM: provide diagnostics on more writeback LDM/STM instructions

Richard Barton richard.barton at arm.com
Wed Oct 23 11:03:52 PDT 2013


Hi Tim

This change has caused an internal fault to be emitted for some of the
unpredictable POP instructions e.g.

echo "popeq    {sp}" | llvm-mc -show-inst -show-encoding -show-inst-operands
-triple armv7

The assertion comes from ARMAsmParser.cpp:5462 in your new diagnostics. 

No prizes for guessing how I found this error...

Could you tweak the patch to remove the abort please?

Thanks
Rich


> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Tim Northover
> Sent: 22 October 2013 20:01
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm] r193185 - ARM: provide diagnostics on more writeback
> LDM/STM instructions
> 
> Author: tnorthover
> Date: Tue Oct 22 14:00:39 2013
> New Revision: 193185
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=193185&view=rev
> Log:
> ARM: provide diagnostics on more writeback LDM/STM instructions
> 
> The set of circumstances where the writeback register is allowed to be in
> the list of registers is rather baroque, but I think this implements them
all
> on the assembly parsing side.
> 
> For disassembly, we still warn about an ARM-mode LDM even if the
> architecture revision is < v7 (the required architecture information isn't
> available). It's a silly instruction anyway, so hopefully no-one will
mind.
> 
> rdar://problem/15223374
> 
> Modified:
>     llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
>     llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
>     llvm/trunk/test/MC/ARM/diagnostics.s
>     llvm/trunk/test/MC/ARM/thumb-diagnostics.s
>     llvm/trunk/test/MC/Disassembler/ARM/invalid-thumbv7.txt
> 
> Modified: llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp?rev=1
> 93185&r1=193184&r2=193185&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp Tue Oct 22
> +++ 14:00:39 2013
> @@ -5416,6 +5416,7 @@ validateInstruction(MCInst &Inst,
>                     "bitfield width must be in range [1,32-lsb]");
>      return false;
>    }
> +  // Notionally handles ARM::tLDMIA_UPD too.
>    case ARM::tLDMIA: {
>      // If we're parsing Thumb2, the .w variant is available and handles
>      // most cases that are normally illegal for a Thumb1 LDM instruction.
> @@ -5444,7 +5445,19 @@ validateInstruction(MCInst &Inst,
> 
>      break;
>    }
> -  case ARM::t2LDMIA_UPD: {
> +  case ARM::LDMIA_UPD:
> +  case ARM::LDMDB_UPD:
> +  case ARM::LDMIB_UPD:
> +  case ARM::LDMDA_UPD:
> +    // ARM variants loading and updating the same register are only
> officially
> +    // UNPREDICTABLE on v7 upwards. Goodness knows what they did
> before.
> +    if (!hasV7Ops())
> +      break;
> +    // Fallthrough
> +  case ARM::t2LDMIA_UPD:
> +  case ARM::t2LDMDB_UPD:
> +  case ARM::t2STMIA_UPD:
> +  case ARM::t2STMDB_UPD: {
>      if (listContainsReg(Inst, 3, Inst.getOperand(0).getReg()))
>        return Error(Operands[4]->getStartLoc(),
>                     "writeback operator '!' not allowed when base register
"
> @@ -5490,10 +5503,19 @@ validateInstruction(MCInst &Inst,
>      break;
>    }
>    case ARM::tSTMIA_UPD: {
> -    bool ListContainsBase;
> -    if (checkLowRegisterList(Inst, 4, 0, 0, ListContainsBase) &&
> !isThumbTwo())
> +    bool ListContainsBase, InvalidLowList;
> +    InvalidLowList = checkLowRegisterList(Inst, 4,
> Inst.getOperand(0).getReg(),
> +                                          0, ListContainsBase);
> +    if (InvalidLowList && !isThumbTwo())
>        return Error(Operands[4]->getStartLoc(),
>                     "registers must be in range r0-r7");
> +
> +    // This would be converted to a 32-bit stm, but that's not valid if
the
> +    // writeback register is in the list.
> +    if (InvalidLowList && ListContainsBase)
> +      return Error(Operands[4]->getStartLoc(),
> +                   "writeback operator '!' not allowed when base register
"
> +                   "in register list");
>      break;
>    }
>    case ARM::tADDrSP: {
> 
> Modified: llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp?r
> ev=193185&r1=193184&r2=193185&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
> (original)
> +++ llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp Tue
> Oct
> +++ 22 14:00:39 2013
> @@ -1203,20 +1203,22 @@ static DecodeStatus DecodeRegListOperand
>                                   uint64_t Address, const void *Decoder) {
>    DecodeStatus S = MCDisassembler::Success;
> 
> -  bool writebackLoad = false;
> -  unsigned writebackReg = 0;
> +  bool NeedDisjointWriteback = false;
> +  unsigned WritebackReg = 0;
>    switch (Inst.getOpcode()) {
> -    default:
> -      break;
> -    case ARM::LDMIA_UPD:
> -    case ARM::LDMDB_UPD:
> -    case ARM::LDMIB_UPD:
> -    case ARM::LDMDA_UPD:
> -    case ARM::t2LDMIA_UPD:
> -    case ARM::t2LDMDB_UPD:
> -      writebackLoad = true;
> -      writebackReg = Inst.getOperand(0).getReg();
> -      break;
> +  default:
> +    break;
> +  case ARM::LDMIA_UPD:
> +  case ARM::LDMDB_UPD:
> +  case ARM::LDMIB_UPD:
> +  case ARM::LDMDA_UPD:
> +  case ARM::t2LDMIA_UPD:
> +  case ARM::t2LDMDB_UPD:
> +  case ARM::t2STMIA_UPD:
> +  case ARM::t2STMDB_UPD:
> +    NeedDisjointWriteback = true;
> +    WritebackReg = Inst.getOperand(0).getReg();
> +    break;
>    }
> 
>    // Empty register lists are not allowed.
> @@ -1226,7 +1228,7 @@ static DecodeStatus DecodeRegListOperand
>        if (!Check(S, DecodeGPRRegisterClass(Inst, i, Address, Decoder)))
>          return MCDisassembler::Fail;
>        // Writeback not allowed if Rn is in the target list.
> -      if (writebackLoad && writebackReg == Inst.end()[-1].getReg())
> +      if (NeedDisjointWriteback && WritebackReg ==
> + Inst.end()[-1].getReg())
>          Check(S, MCDisassembler::SoftFail);
>      }
>    }
> 
> Modified: llvm/trunk/test/MC/ARM/diagnostics.s
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/MC/ARM/diagnostics.s?rev=193185&r1=193184&
> r2=193185&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/test/MC/ARM/diagnostics.s (original)
> +++ llvm/trunk/test/MC/ARM/diagnostics.s Tue Oct 22 14:00:39 2013
> @@ -429,3 +429,10 @@
> 
>          bkpteq #7
>  @ CHECK-ERRORS: error: instruction 'bkpt' is not predicable, but
condition
> code specified
> +
> +        ldm r2!, {r2, r3}
> +        ldmdb r2!, {r2, r3}
> +        ldmda r2!, {r2, r3}
> +@ CHECK-ERRORS: error: writeback operator '!' not allowed when base
> +register in register list @ CHECK-ERRORS: error: writeback operator '!'
> +not allowed when base register in register list @ CHECK-ERRORS: error:
> +writeback operator '!' not allowed when base register in register list
> 
> Modified: llvm/trunk/test/MC/ARM/thumb-diagnostics.s
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/MC/ARM/thumb-
> diagnostics.s?rev=193185&r1=193184&r2=193185&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/test/MC/ARM/thumb-diagnostics.s (original)
> +++ llvm/trunk/test/MC/ARM/thumb-diagnostics.s Tue Oct 22 14:00:39
> 2013
> @@ -57,6 +57,8 @@ error: invalid operand for instruction
>          ldm r2!, {r5, r8}
>          ldm r2, {r5, r7}
>          ldm r2!, {r2, r3, r4}
> +        ldm r2!, {r2, r3, r4, r10}
> +        ldmdb r2!, {r2, r3, r4}
>  @ CHECK-ERRORS: error: registers must be in range r0-r7
>  @ CHECK-ERRORS:         ldm r2!, {r5, r8}
>  @ CHECK-ERRORS:                  ^
> @@ -66,6 +68,12 @@ error: invalid operand for instruction  @ CHECK-
> ERRORS: error: writeback operator '!' not allowed when base register in
> register list
>  @ CHECK-ERRORS:         ldm r2!, {r2, r3, r4}
>  @ CHECK-ERRORS:               ^
> +@ CHECK-ERRORS-V8: error: writeback operator '!' not allowed when base
> register in register list
> +@ CHECK-ERRORS-V8:         ldm r2!, {r2, r3, r4, r10}
> +@ CHECK-ERRORS-V8:               ^
> +@ CHECK-ERRORS-V8: error: writeback operator '!' not allowed when base
> register in register list
> +@ CHECK-ERRORS-V8:         ldmdb r2!, {r2, r3, r4}
> +@ CHECK-ERRORS-V8:                 ^
> 
>  @ Invalid writeback and register lists for PUSH/POP
>          pop {r1, r2, r10}
> @@ -81,12 +89,20 @@ error: invalid operand for instruction  @ Invalid
> writeback and register lists for STM
>          stm r1, {r2, r6}
>          stm r1!, {r2, r9}
> +        stm r2!, {r2, r9}
> +        stmdb r2!, {r0, r2}
>  @ CHECK-ERRORS: error: instruction requires: thumb2
>  @ CHECK-ERRORS:         stm r1, {r2, r6}
>  @ CHECK-ERRORS:         ^
>  @ CHECK-ERRORS: error: registers must be in range r0-r7
>  @ CHECK-ERRORS:         stm r1!, {r2, r9}
>  @ CHECK-ERRORS:                  ^
> +@ CHECK-ERRORS-V8: error: writeback operator '!' not allowed when base
> register in register list
> +@ CHECK-ERRORS-V8:         stm r2!, {r2, r9}
> +@ CHECK-ERRORS-V8:                  ^
> +@ CHECK-ERRORS-V8: error: writeback operator '!' not allowed when base
> register in register list
> +@ CHECK-ERRORS-V8:         stmdb r2!, {r0, r2}
> +@ CHECK-ERRORS-V8:                  ^
> 
>  @ Out of range immediates for LSL instruction.
>          lsls r4, r5, #-1
> 
> Modified: llvm/trunk/test/MC/Disassembler/ARM/invalid-thumbv7.txt
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/MC/Disassembler/ARM/invalid-
> thumbv7.txt?rev=193185&r1=193184&r2=193185&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/test/MC/Disassembler/ARM/invalid-thumbv7.txt (original)
> +++ llvm/trunk/test/MC/Disassembler/ARM/invalid-thumbv7.txt Tue Oct
> 22
> +++ 14:00:39 2013
> @@ -389,3 +389,19 @@
>  [0x80 0xf9 0x30 0x0b]
>  # CHECK: invalid instruction encoding
>  # CHECK-NEXT: [0x80 0xf9 0x30 0x0b]
> +
> +
> +#----------------------------------------------------------------------
> +--------
> +# Unpredictable STMs
> +#----------------------------------------------------------------------
> +--------
> +
> +# 32-bit Thumb STM instructions cannot have a writeback register which
> +appears # in the list.
> +
> +[0xa1,0xe8,0x07,0x04]
> +# CHECK: warning: potentially undefined instruction encoding #
> +CHECK-NEXT: [0xa1,0xe8,0x07,0x04]
> +
> +[0x21,0xe9,0x07,0x04]
> +# CHECK: warning: potentially undefined instruction encoding #
> +CHECK-NEXT: [0x21,0xe9,0x07,0x04]
> 
> 
> _______________________________________________
> 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