[llvm-dev] PR36144: X86 Intel syntax and masm flavor

[Reid Kleckner via llvm-dev]

After looking more closely at the patch in question, I again think it
should be reverted.

The patch sets a boolean to indicate that inline asm is being parsed in
three places now:
1. true when parsing intel inline asm
2. true when .intel_syntax directives are encountered (BUG!)
3. false when .att_syntax is encountered

We should only set this to true when parsing *inline asm*, clearly. I sent
my second email after I saw place 1 when re-reading the patch and thought,
oh, everything is working as intended. I'll go ahead and do it, since it is
clearly wrong. .intel_syntax should not imply IsParsingInlineAsm.

On Mon, Oct 22, 2018 at 1:13 PM Gerolf Hoflehner <ghoflehner at apple.com>
wrote:

>
>
> On Sep 12, 2018, at 1:48 PM, Reid Kleckner via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Sorry, I spoke too soon. This only happens for intel style inline assembly
> in LLVM IR. I don't have a good suggestion.
>
>
> Why is it relevant that the issue is contained? The 0b support in MS asm
> shouldn’t break the general intel assembly syntax.
>
>
> On Wed, Sep 12, 2018 at 1:44 PM Reid Kleckner <rnk at google.com> wrote:
>
>> I think we should revert r301390 just on principle from looking at the
>> code. If I understand correctly, it flips the bit for "is parsing inline
>> asm" to true when encountering a plain .intel_syntax directive. That's just
>> wrong.
>>
>> On Wed, Sep 12, 2018 at 6:34 AM Francis Visoiu Mistrih via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Hi,
>>>
>>> We have a significant regression since llvm 5.0.0 in the x86 assembler.
>>>
>>> The following snippets fail:
>>>
>>> 1)
>>>
>>> .intel_syntax
>>> 0:
>>>   jmp 0b
>>>
>>> 2)
>>>
>>> .intel_syntax
>>>   and edi, 0b010101
>>>
>>> when running through `llvm-mc -arch x86-64`.
>>>
>>> This regression was introduced in r301390, which was driven by PR27884.
>>>
>>> I think https://llvm.org/PR36144 describes this very well, and I think
>>> we should
>>> get this fixed, since it's a pretty basic thing to support in the
>>> assembler.
>>>
>>> Here are a few solutions to this:
>>>
>>> 1) Introduce a new asm dialect/flavor/style to assemble masm files.
>>>
>>> 2) Only set the flags based on the target triple. Also suggested in
>>> PR27884.
>>>
>>> 3) Only set the flags based on a new command line flag.
>>>
>>> Let me know if any other solution comes to mind.
>>>
>>> While we get this issue fixed, is it reasonable to revert r301390?
>>>
>>> Thanks,
>>>
>>> --
>>> Francis
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181022/30254ce6/attachment.html>