Home Original page

[PATCH] i386: Only check suffix in instruction mnemonic

H.J. Lu hjl.tools@gmail.com
Mon Nov 11 17:04:00 GMT 2019 
On Mon, Nov 11, 2019 at 3:06 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.11.2019 16:53, H.J. Lu wrote:
> > On Thu, Nov 7, 2019 at 11:41 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 07.11.2019 22:52,  H.J. Lu  wrote:
> >>> Another problem is Intel syntax sets instruction suffix from operand size:
> >>
> >> An unrelated one, but yes.
> >>
> >>> [hjl@gnu-cfl-1 gas]$ cat 2.s
> >>> .intel_syntax noprefix
> >>> call WORD PTR [rax]
> >>> [hjl@gnu-cfl-1 gas]$ as -mintel64 -o 2.o 2.s
> >>> 2.s: Assembler messages:
> >>> 2.s:2: Error: invalid instruction suffix for `call'
> >>> [hjl@gnu-cfl-1 gas]$
> >>>
> >>> It is operand size mismatch, not invalid instruction suffix.  In case
> >>> of CMPSD and
> >>> and MOVSD,the 'l' suffix is invalid, not DWORD on memory operand.  But suffix
> >>> check treats them the same.  This makes Intel syntax more complicated.   Should
> >>> Intel syntax disallow suffix in mnemonic?  It will make Intel syntax
> >>> closer to SDM.
> >>
> >> There are some suffixes that need honoring. The original authors
> >> of the code apparently decided that it's better to accept some
> >> stray/bogus suffixes than to further complicate the code. I guess
> >> once we have the No_*Suf attributes put straight (as mentioned by
> >> Michael the other day, matching plans I've been having), then we
> >> could see about further cleaning up Intel syntax behavior here.
> >>
> >
> > I am checking in this.
> >
> > We should check suffix in instruction mnemonic when matching instruction.
> > In Intel syntax, normally we check for memory operand size.  But the same
> > mnemonic with 2 different encodings can have the same memory operand
> > size and i.suffix is set to LONG_DOUBLE_MNEM_SUFFIX from memory operand
> > size in Intel syntax to distinguish them.  When there is no suffix in
> > mnemonic, we check LONG_DOUBLE_MNEM_SUFFIX in i.suffix for mnemonic
> > suffix.
>
> So even after quite a bit of checking and thinking my gut feeling
> still is that this may have broken some corner cases. But I can't
> come up with a specific example, so maybe all is well.
>
> On the positive side this fixes MOVDIRI handling: Previously only
> the operand-size less cases below would have been accepted, whereas
> now all 6 valid ones remain without diagnostic.
>
> I suspect though that your change should have been accompanied
> with further IgnoreSize dropping, as I think quite a few that we
> still have in there are no longer needed now.

I agree.

> Btw, would you mind me putting in the testsuite parts of the
> alternative patches I had sent for this PR?
>
> Jan
>
>         movdiri [rcx], eax
>         movdiri dword ptr [rcx], eax
>         movdiri qword ptr [rcx], eax
>
>         movdiri [rcx], rax
>         movdiri dword ptr [rcx], rax
>         movdiri qword ptr [rcx], rax
>
>         .code32
>         movdiri [ecx], eax
>         movdiri dword ptr [ecx], eax
>         movdiri qword ptr [ecx], eax
>

Can you submit a patch?

Thanks.

-- 
H.J.
More information about the Binutils mailing list