Re: [PATCH 2/6] x86-64: Intel64 adjustments for conditional jumps

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Mar 6, 2020 at 6:53 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 06.03.2020 15:39, H.J. Lu wrote:
> > On Fri, Mar 6, 2020 at 12:12 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> Just like for unconditional direct JMP, AMD and Intel differ in their
> >> handling. Mirror JMP handling to Jcc.
> >>
> >> gas/
> >> 2020-03-XX  Jan Beulich  <jbeulich@suse.com>
> >>
> >>         * testsuite/gas/i386/x86-64-branch-2.s,
> >>         testsuite/gas/i386/x86-64-branch-3.s: Add Jcc cases.
> >>         * testsuite/gas/i386/ilp32/x86-64-branch.d,
> >>         testsuite/gas/i386/opcode-suffix.d,
> >>         testsuite/gas/i386/x86-64-branch-2.d,
> >>         testsuite/gas/i386/x86-64-branch-3.d,
> >>         testsuite/gas/i386/x86-64-branch.d: Adjust expectations.
> >>
> >> opcodes/
> >> 2020-03-XX  Jan Beulich  <jbeulich@suse.com>
> >>
> >>         * i386-dis.c (safe-ctype.h): Include.
> >>         (X86_64_0F8x): New enumerator.
> >>         (dis386): Extend comment ahead of it.
> >>         (dis386_twobyte): Vector Jcc to X86_64_0F8x.
> >>         (condition_code): New.
> >>         (x86_64_table): Add X86_64_0F8x entry.
> >>         (print_insn): Set condition_code. Move advancing of codep after
> >>         it.
> >>         (putop): Handle two-char escape case for 'C'. Handle 'C' prefix
> >>         case for 'P' and '@'.
> >>         * i386-opc.tbl (j<cc>): Split into AMD64 and Intel64 variants.
> >>         * i386-tbl.h: Re-generate.
> >> ---
> >> I wonder if the suffix handling done here wouldn't also be the more
> >> suitable one for JMP and CALL. In particular the 'q' suffix printed
> >> unconditionally in 64-bit mode is more of a problem than helpful imo.
> >>
> >> --- a/gas/testsuite/gas/i386/ilp32/x86-64-branch.d
> >> +++ b/gas/testsuite/gas/i386/ilp32/x86-64-branch.d
> >> @@ -22,7 +22,7 @@ Disassembly of section .text:
> >>  [      ]*[a-f0-9]+:    e9 00 00 00 00          jmpq   0x24     20: R_X86_64_PC32       \*ABS\*\+0x10003c
> >>  [      ]*[a-f0-9]+:    66 e8 00 00 00 00       data16 callq 0x2a       26: R_X86_64_PLT32      foo-0x4
> >>  [      ]*[a-f0-9]+:    66 e9 00 00 00 00       data16 jmpq 0x30        2c: R_X86_64_PLT32      foo-0x4
> >> -[      ]*[a-f0-9]+:    66 0f 82 00 00 00 00    data16 jb 0x37  33: R_X86_64_PLT32      foo-0x4
> >> +[      ]*[a-f0-9]+:    66 0f 82 00 00 00 00    data16 jbq 0x37 33: R_X86_64_PLT32      foo-0x4
> >>  [      ]*[a-f0-9]+:    66 c3                   data16 retq *
> >>  [      ]*[a-f0-9]+:    66 c2 08 00             data16 retq \$0x8
> >>  [      ]*[a-f0-9]+:    ff d0                   callq  \*%rax
> >
> > I think it is a very bad idea to add suffix to jcc.
>
> Well, do you have an alternative suggestion, also in line with
> JMP then? (See the somewhat related post-commit-message remark

Since assembly doesn't require `q' suffix, can we drop it from disassembler?

> above as well.) I'd like to note that a suffix gets put there
> _only_ if there's also a data16 prefix (to be able to tell
> apart the different cases).



-- 
H.J.