[PATCH] disabled switching ARM modes in inline asm

[Greg Fitzgerald]

David, thanks for the review.

> 'isParsingInlineAsm()' vs. 'getInlineAsmKind() == MAK_RAW'? The named method
> looks more readable to me.

Oddly, the existing method isParsingInlineAsm() actually means
isParsing MS-style Asm.  So to make it more readable, I'd change
isParsingInlineAsm() to isParsingMsInlineAsm() and implement
isParsingInlineAsm() as "return getInlineAsmKind() != MAK_Raw".  The
isParsingMsInlineAsm() calls seems redundant to me, and redefining
isParsingInlineAsm() is risky.  With the current patch, if I missed a
call site (for example, some dependency outside the llvm repo) it
would fail to compile, which I feel is better than silently behaving
differently.



> You changed the name of 'isParsingInlineAsm()' to 'getInlineAsmKind()' in
> the header, but I do not see any updates to call sites for that method. Is
> it actually used anywhere?

Yes, see X86AsmParser.cpp


Thanks,
Greg


On Wed, Dec 11, 2013 at 2:56 PM, David Peixotto <dpeixott at codeaurora.org> wrote:
> Greg,
>
> A few questions/comments.
>
> 1. Would it be better to have convinence methods on the parser for querying
> inlineasm state rather than exposing the enum directly? Something like
> 'isParsingInlineAsm()' vs. 'getInlineAsmKind() == MAK_RAW'? The named method
> looks more readable to me.
>
> 2. You changed the name of 'isParsingInlineAsm()' to 'getInlineAsmKind()' in
> the header, but I do not see any updates to call sites for that method. Is
> it actually used anywhere? And if not do we still need the distinction
> between the inline asm kinds.
>
> -David
>
>
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>> bounces at cs.uiuc.edu] On Behalf Of Renato Golin
>> Sent: Wednesday, December 11, 2013 1:15 PM
>> To: grosbach at apple.com; amara.emerson at arm.com; garious at gmail.com
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [PATCH] disabled switching ARM modes in inline asm
>>
>>
>>   LGTM
>>
>> http://llvm-reviews.chandlerc.com/D2255
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>