<div dir="ltr">On Tue, Dec 23, 2014 at 12:35 PM, Bob Wilson <span dir="ltr"><<a href="mailto:bob.wilson@apple.com" target="_blank">bob.wilson@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">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.</blockquote><div><br></div><div>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.</div><div><br></div><div>Thanks for pointing that out!</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">
> On Dec 17, 2014, at 8:17 AM, Saleem Abdulrasool <<a href="mailto:compnerd@compnerd.org">compnerd@compnerd.org</a>> wrote:<br>
><br>
> Author: compnerd<br>
> Date: Wed Dec 17 10:17:44 2014<br>
> New Revision: 224432<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224432&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224432&view=rev</a><br>
> Log:<br>
> ARM: correct an off-by-one in an assert<br>
><br>
> The assert was off-by-one, resulting in failures for valid input.<br>
><br>
> Thanks to Asiri Rathnayake for pointing out the failure!<br>
><br>
> Modified:<br>
>    llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp<br>
>    llvm/trunk/test/MC/ARM/arm-store-deprecated.s<br>
><br>
> Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp?rev=224432&r1=224431&r2=224432&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp?rev=224432&r1=224431&r2=224432&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp (original)<br>
> +++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp Wed Dec 17 10:17:44 2014<br>
> @@ -77,7 +77,10 @@ static bool getITDeprecationInfo(MCInst<br>
><br>
> static bool getARMStoreDeprecationInfo(MCInst &MI, MCSubtargetInfo &STI,<br>
>                                        std::string &Info) {<br>
> -  assert(MI.getNumOperands() > 4 && "expected >4 arguments");<br>
> +  if (STI.getFeatureBits() & llvm::ARM::ModeThumb)<br>
> +    return false;<br>
> +<br>
> +  assert(MI.getNumOperands() >= 4 && "expected >= 4 arguments");<br>
>   for (unsigned OI = 4, OE = MI.getNumOperands(); OI < OE; ++OI) {<br>
>     assert(MI.getOperand(OI).isReg() && "expected register");<br>
>     if (MI.getOperand(OI).getReg() == ARM::SP ||<br>
><br>
> Modified: llvm/trunk/test/MC/ARM/arm-store-deprecated.s<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/arm-store-deprecated.s?rev=224432&r1=224431&r2=224432&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/arm-store-deprecated.s?rev=224432&r1=224431&r2=224432&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/MC/ARM/arm-store-deprecated.s (original)<br>
> +++ llvm/trunk/test/MC/ARM/arm-store-deprecated.s Wed Dec 17 10:17:44 2014<br>
> @@ -145,3 +145,9 @@ push:<br>
> @ CHECK: push {sp}<br>
> @ CHECK: ^<br>
><br>
> +     .global single<br>
> +     .type single,%function<br>
> +single:<br>
> +     stmdaeq r0, {r0}<br>
> +@ CHECK-NOT: warning<br>
> +<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</div></div>