[llvm] r224432 - ARM: correct an off-by-one in an assert

[Bob Wilson]

The commit messages refers to changing the assert, but you also added a check for Thumb mode. Was that intentional? It doesn’t seem to be necessary, since this function is only referenced for ARM-mode instructions in the .td files. The same Thumb check was also carried over to the getARMLoadDeprecationInfo function that was added after this, and I’m not sure it’s necessary there, either.

> On Dec 17, 2014, at 8:17 AM, Saleem Abdulrasool <compnerd at compnerd.org> wrote:
> 
> Author: compnerd
> Date: Wed Dec 17 10:17:44 2014
> New Revision: 224432
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=224432&view=rev
> Log:
> ARM: correct an off-by-one in an assert
> 
> The assert was off-by-one, resulting in failures for valid input.
> 
> Thanks to Asiri Rathnayake for pointing out the failure!
> 
> Modified:
>    llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
>    llvm/trunk/test/MC/ARM/arm-store-deprecated.s
> 
> Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp?rev=224432&r1=224431&r2=224432&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp Wed Dec 17 10:17:44 2014
> @@ -77,7 +77,10 @@ static bool getITDeprecationInfo(MCInst
> 
> static bool getARMStoreDeprecationInfo(MCInst &MI, MCSubtargetInfo &STI,
>                                        std::string &Info) {
> -  assert(MI.getNumOperands() > 4 && "expected >4 arguments");
> +  if (STI.getFeatureBits() & llvm::ARM::ModeThumb)
> +    return false;
> +
> +  assert(MI.getNumOperands() >= 4 && "expected >= 4 arguments");
>   for (unsigned OI = 4, OE = MI.getNumOperands(); OI < OE; ++OI) {
>     assert(MI.getOperand(OI).isReg() && "expected register");
>     if (MI.getOperand(OI).getReg() == ARM::SP ||
> 
> Modified: llvm/trunk/test/MC/ARM/arm-store-deprecated.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/arm-store-deprecated.s?rev=224432&r1=224431&r2=224432&view=diff
> ==============================================================================
> --- llvm/trunk/test/MC/ARM/arm-store-deprecated.s (original)
> +++ llvm/trunk/test/MC/ARM/arm-store-deprecated.s Wed Dec 17 10:17:44 2014
> @@ -145,3 +145,9 @@ push:
> @ CHECK: push {sp}
> @ CHECK: ^
> 
> +	.global single
> +	.type single,%function
> +single:
> +	stmdaeq r0, {r0}
> +@ CHECK-NOT: warning
> +
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits