V2 [PATCH] x86: Add {disp16} pseudo prefix

[H.J. Lu]

On Wed, Jul 29, 2020 at 3:53 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> 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.
>

Here is the updated patch.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Add-disp16-pseudo-prefix.patch
Type: text/x-patch
Size: 28654 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20200729/901c6050/attachment-0001.bin>