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

[Saleem Abdulrasool]

On Tue, Dec 23, 2014 at 12:35 PM, Bob Wilson <bob.wilson at apple.com> wrote:

> 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.


It was added as I was paranoid about the misuse, but it wasn't supposed to
be part of the same change.  I think that you are right that it is
unnecessary, but I still think that having the check there is nice to
prevent misuse.  However, that purpose is much better served as an
assertion.  I've changed both instances to an assert in SVN r224825.

Thanks for pointing that out!


> > 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
>
>


-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141224/d47914b9/attachment.html>