[PATCH] x86: Add {disp16} pseudo prefix

[H.J. Lu]

On Wed, Jul 29, 2020 at 1:26 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 28.07.2020 22:30, H.J. Lu wrote:
> > On Tue, Jul 28, 2020 at 12:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Tue, Jul 28, 2020 at 11:43 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>
> >>> On 28.07.2020 01:23, H.J. Lu via Binutils wrote:
> >>>> Since (%bp)/(%ebp)/(%rbp) are encoded as 0(%bp)/0(%ebp)/0(%rbp), use
> >>>> disp32/disp16 on 0(%bp)/0(%ebp)/0(%rbp) for {disp32}.
> >>>
> >>> Same for (%r13d) / (%r13) afaict?
> >>
> >> Yes. {disp32} works on (%r13d) / (%r13) now:
> >>
> >> [hjl@gnu-cfl-2 testsuite]$ cat x.s
> >> movb (%r13),%al
> >> {disp8} movb (%r13),%al
> >> {disp32} movb (%r13),%al
> >>
> >> movb (%r13d),%al
> >> {disp8} movb (%r13d),%al
> >> {disp32} movb (%r13d),%al
> >> [hjl@gnu-cfl-2 testsuite]$ gcc -c x.s
> >> [hjl@gnu-cfl-2 testsuite]$ objdump -dw x.o
> >>
> >> x.o:     file format elf64-x86-64
> >>
> >>
> >> Disassembly of section .text:
> >>
> >> 0000000000000000 <.text>:
> >>     0: 41 8a 45 00          mov    0x0(%r13),%al
> >>     4: 41 8a 45 00          mov    0x0(%r13),%al
> >>     8: 41 8a 45 00          mov    0x0(%r13),%al
> >>     c: 67 41 8a 45 00        mov    0x0(%r13d),%al
> >>    11: 67 41 8a 45 00        mov    0x0(%r13d),%al
> >>    16: 67 41 8a 45 00        mov    0x0(%r13d),%al
> >> [hjl@gnu-cfl-2 testsuite]$ ../as-new -o x.o x.s
> >> [hjl@gnu-cfl-2 testsuite]$ objdump -dw x.o
> >>
> >> x.o:     file format elf64-x86-64
> >>
> >>
> >> Disassembly of section .text:
> >>
> >> 0000000000000000 <.text>:
> >>     0: 41 8a 45 00          mov    0x0(%r13),%al
> >>     4: 41 8a 45 00          mov    0x0(%r13),%al
> >>     8: 41 8a 85 00 00 00 00 mov    0x0(%r13),%al
> >>     f: 67 41 8a 45 00        mov    0x0(%r13d),%al
> >>    14: 67 41 8a 45 00        mov    0x0(%r13d),%al
> >>    19: 67 41 8a 85 00 00 00 00 mov    0x0(%r13d),%al
> >> [hjl@gnu-cfl-2 testsuite]$
> >>
> >>
> >>>> Note: Since there is no disp32 on 0(%bp), use disp16 instead.
> >>>
> >>> What use is it to fix the special case of (%bp) when the more general
> >>> case ((%bx), (%si), etc) doesn't work? I anyway think that instead of
> >>> ...
> >>>
> >>>> --- a/gas/config/tc-i386.c
> >>>> +++ b/gas/config/tc-i386.c
> >>>> @@ -8151,7 +8151,12 @@ build_modrm_byte (void)
> >>>>                      if (operand_type_check (i.types[op], disp) == 0)
> >>>>                        {
> >>>>                          /* fake (%bp) into 0(%bp)  */
> >>>> -                       i.types[op].bitfield.disp8 = 1;
> >>>> +                       if (i.disp_encoding == disp_encoding_32bit)
> >>>> +                         /* NB: Use disp16 since there is no disp32
> >>>> +                            in 16-bit mode.  */
> >>>> +                         i.types[op].bitfield.disp16 = 1;
> >>>> +                       else
> >>>> +                         i.types[op].bitfield.disp8 = 1;
> >>>>                          fake_zero_displacement = 1;
> >>>>                        }
> >>>
> >>> ... the comment you add here, support for {disp16} should be added.
> >>>
> >>
> >> I will add {disp16} to master branch.
> >>
> >
> > Add {disp16} pseudo prefix and replace {disp32} pseudo prefix with
> > {disp16} in 16-bit mode test.  Check invalid {disp16}/{disp32} pseudo
> > prefixes.
>
> The inval-pseudo test is a 32-bit one; you shouldn't use .code64 there,
> or you'll cause FAILs on 32-bit only builds.

I will fix it.

> > Note: {disp16} can be also used on branches in 32-bit mode.
>
> But you don't add any tests to this effect, so it's hard to see what
> exactly this means. To be honest I'm not sure this is helpful: A means
> to widen the default displacement may be useful, but one to narrow not
> just the displacement, but also the resulting new IP?

{disp16} can be used to generate "branch rel16".  But implementation
may be non-trivial.

> I was also wanting to ask that you group the new pseudo prefix with
> its sibling ones in the opcode table, but I realize the use of
> hard coded numbers makes this cumbersome. Time to do away with that?

I will take a look.

> I also have a tangential question: Shouldn't e.g.
>
>         {disp8} vmovaps %xmm0,128(%eax)
>
> be taken as a request to use EVEX encoding, to satisfy the pseudo
> prefix? Unless {vex} was also specified, at which point things
> become "interesting" (but the way they're documented I think
> {vex} has to have more wight here).
>

{disp8} is a hint for VEX.  It shouldn't change to EVEX.

-- 
H.J.