[PATCH] disabled switching ARM modes in inline asm

Jim Grosbach grosbach at apple.com
Wed Dec 11 17:26:23 PST 2013


On Dec 11, 2013, at 5:19 PM, David Peixotto <dpeixott at codeaurora.org> wrote:

>> 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.
> 
> I would be slightly in favor of having both isParsingInlineAsm() (needed for
> arm) and isParsingMSInlineAsm() (needed for x86), but I understand the
> concern about changing the meaning of isParsingInlineAsm().

MS style inline asm is currently x86 only, but conceptually that need not be true. There’s been some talk about what it would take to make it work for ARM, too, for example.

> 
>>> 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
> 
> Sorry, I asked about the wrong function. I meant to ask about
> 'setParsingInlineAsm()'. I did not see any calls to that function. Maybe
> they are in clang?

I believe so, yes.

-Jim



More information about the llvm-commits mailing list