Fix for pr13145: Naming a function like a register name confuses the asm parser

Chad Rosier mcrosier at apple.com
Tue Mar 19 16:52:57 PDT 2013


Hi Stepan,
I went ahead and applied your patch in r177463.

 Chad

On Mar 18, 2013, at 9:41 AM, Chad Rosier <mcrosier at apple.com> wrote:

> 
> On Mar 18, 2013, at 1:32 AM, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:
> 
>> Hello Chad.
>> Sorry about that, a little bit confused by ogre3d style. Variable name was fixed. I also replaced the comment.
>> Unittest is applied.
> 
> Great, thanks.
> 
> +    // If we've seen a branch mnemonic, the next operand must be a label then.
> +    // Even if it named as register. So "br r1" would mean:
> +    // branch to "r1" label.
> 
> Perhaps this comment:
> 
> // If we've seen a branch mnemonic, the next operand must be a label.
> // This is true even if the label is a register name.  So "br r1" means
> // branch to label "r1."
> 
> If you have commit access, please go ahead and commit the patch.  Otherwise, let me know and I'll commit on you behalf.
> 
>  Chad
> 
>> -Stepan.
>> 
>> Chad Rosier wrote:
>>> Hi Stepan,
>>> Here are a few nits:
>>> 
>>> When writing comments, write them as English prose, which means they
>>> should use proper capitalization, punctuation, etc.
>>> 
>>> +    // Want label whatever happened...
>>> +    bool wantLabelOnly = Mnemonic == "b" || Mnemonic == "bl";
>>> 
>>> Maybe the comment could be something like, "If we've seen a branch
>>> mnemonic, the next operand must be a label."
>>> 
>>> You could also rename wantLabelOnly to ExpectLabel or just put the
>>> conditional in the if statement; the comment should make clear the
>>> condition of the if statement.
>>> 
>>> Most importantly, please add a test case before resubmitting to the list.
>>> 
>>>  Chad
>> 
>> <pr13145.patch>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130319/f33cd167/attachment.html>


More information about the llvm-commits mailing list