public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH v3 6/7] x86-64: allow HLE store of accumulator to absolute 32-bit address
Date: Tue, 11 Oct 2022 10:50:04 -0700	[thread overview]
Message-ID: <CAMe9rOqEf+zmzaxR7brXYWKot9pZgygBL2Hncaw-S+vdf8VjDg@mail.gmail.com> (raw)
In-Reply-To: <e4692495-7d3d-074d-14f9-364d4a9a998c@suse.com>

On Wed, Oct 5, 2022 at 12:25 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> In commit 1212781b35c9 ("ix86: allow HLE store of accumulator to
> absolute address") I was wrong to exclude 64-bit code. Dropping the
> check also leads to better diagnostics in 64-bit code ("MOV", after
> all, isn't invalid with "XRELEASE").
>
> While there also limit the amount of further checks done: The operand
> type checks that were there were effectively redundant with other ones
> anyway, plus it's quite fine to also have "xrelease mov <disp>, %eax"
> look for the next MOV template (in fact again also improving
> diagnostics).
> ---
> v2: New.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -6819,12 +6819,9 @@ match_template (char mnem_suffix)
>             continue;
>           /* xrelease mov %eax, <disp> is another special case. It must not
>              match the accumulator-only encoding of mov.  */
> -         if (flag_code != CODE_64BIT
> -             && i.hle_prefix
> +         if (i.hle_prefix
>               && t->base_opcode == 0xa0
> -             && t->opcode_modifier.opcodespace == SPACE_BASE
> -             && i.types[0].bitfield.instance == Accum
> -             && (i.flags[1] & Operand_Mem))
> +             && t->opcode_modifier.opcodespace == SPACE_BASE)
>             continue;
>           /* Fall through.  */
>
> --- a/gas/testsuite/gas/i386/x86-64-hle-intel.d
> +++ b/gas/testsuite/gas/i386/x86-64-hle-intel.d
> @@ -425,6 +425,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 20 01             lock xacquire and BYTE PTR \[rcx\],al
>  [      ]*[a-f0-9]+:    f0 f3 20 01             lock xrelease and BYTE PTR \[rcx\],al
>  [      ]*[a-f0-9]+:    f3 88 01                xrelease mov BYTE PTR \[rcx\],al
> +[      ]*[a-f0-9]+:    f3 88 04 25 78 56 34 12         xrelease mov BYTE PTR (ds:)?0x12345678,al
> +[      ]*[a-f0-9]+:    67 f3 88 04 25 21 43 65 87      xrelease mov BYTE PTR \[eiz\*1\+0x87654321\],al
>  [      ]*[a-f0-9]+:    f2 f0 08 01             xacquire lock or BYTE PTR \[rcx\],al
>  [      ]*[a-f0-9]+:    f2 f0 08 01             xacquire lock or BYTE PTR \[rcx\],al
>  [      ]*[a-f0-9]+:    f3 f0 08 01             xrelease lock or BYTE PTR \[rcx\],al
> @@ -476,6 +478,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 66 21 01          lock xacquire and WORD PTR \[rcx\],ax
>  [      ]*[a-f0-9]+:    f0 f3 66 21 01          lock xrelease and WORD PTR \[rcx\],ax
>  [      ]*[a-f0-9]+:    66 f3 89 01             xrelease mov WORD PTR \[rcx\],ax
> +[      ]*[a-f0-9]+:    66 f3 89 04 25 78 56 34 12      xrelease mov WORD PTR (ds:)?0x12345678,ax
> +[      ]*[a-f0-9]+:    67 66 f3 89 04 25 21 43 65 87   xrelease mov WORD PTR \[eiz\*1\+0x87654321\],ax
>  [      ]*[a-f0-9]+:    66 f2 f0 09 01          xacquire lock or WORD PTR \[rcx\],ax
>  [      ]*[a-f0-9]+:    66 f2 f0 09 01          xacquire lock or WORD PTR \[rcx\],ax
>  [      ]*[a-f0-9]+:    66 f3 f0 09 01          xrelease lock or WORD PTR \[rcx\],ax
> @@ -527,6 +531,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 21 01             lock xacquire and DWORD PTR \[rcx\],eax
>  [      ]*[a-f0-9]+:    f0 f3 21 01             lock xrelease and DWORD PTR \[rcx\],eax
>  [      ]*[a-f0-9]+:    f3 89 01                xrelease mov DWORD PTR \[rcx\],eax
> +[      ]*[a-f0-9]+:    f3 89 04 25 78 56 34 12         xrelease mov DWORD PTR (ds:)?0x12345678,eax
> +[      ]*[a-f0-9]+:    67 f3 89 04 25 21 43 65 87      xrelease mov DWORD PTR \[eiz\*1\+0x87654321\],eax
>  [      ]*[a-f0-9]+:    f2 f0 09 01             xacquire lock or DWORD PTR \[rcx\],eax
>  [      ]*[a-f0-9]+:    f2 f0 09 01             xacquire lock or DWORD PTR \[rcx\],eax
>  [      ]*[a-f0-9]+:    f3 f0 09 01             xrelease lock or DWORD PTR \[rcx\],eax
> @@ -578,6 +584,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 48 21 01          lock xacquire and QWORD PTR \[rcx\],rax
>  [      ]*[a-f0-9]+:    f0 f3 48 21 01          lock xrelease and QWORD PTR \[rcx\],rax
>  [      ]*[a-f0-9]+:    f3 48 89 01             xrelease mov QWORD PTR \[rcx\],rax
> +[      ]*[a-f0-9]+:    f3 48 89 04 25 78 56 34 12      xrelease mov QWORD PTR (ds:)?0x12345678,rax
> +[      ]*[a-f0-9]+:    67 f3 48 89 04 25 21 43 65 87   xrelease mov QWORD PTR \[eiz\*1\+0x87654321\],rax
>  [      ]*[a-f0-9]+:    f2 f0 48 09 01          xacquire lock or QWORD PTR \[rcx\],rax
>  [      ]*[a-f0-9]+:    f2 f0 48 09 01          xacquire lock or QWORD PTR \[rcx\],rax
>  [      ]*[a-f0-9]+:    f3 f0 48 09 01          xrelease lock or QWORD PTR \[rcx\],rax
> --- a/gas/testsuite/gas/i386/x86-64-hle.d
> +++ b/gas/testsuite/gas/i386/x86-64-hle.d
> @@ -424,6 +424,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 20 01             lock xacquire and %al,\(%rcx\)
>  [      ]*[a-f0-9]+:    f0 f3 20 01             lock xrelease and %al,\(%rcx\)
>  [      ]*[a-f0-9]+:    f3 88 01                xrelease mov %al,\(%rcx\)
> +[      ]*[a-f0-9]+:    f3 88 04 25 78 56 34 12         xrelease mov %al,0x12345678
> +[      ]*[a-f0-9]+:    67 f3 88 04 25 21 43 65 87      xrelease mov %al,0x87654321\(,%eiz,1\)
>  [      ]*[a-f0-9]+:    f2 f0 08 01             xacquire lock or %al,\(%rcx\)
>  [      ]*[a-f0-9]+:    f2 f0 08 01             xacquire lock or %al,\(%rcx\)
>  [      ]*[a-f0-9]+:    f3 f0 08 01             xrelease lock or %al,\(%rcx\)
> @@ -475,6 +477,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 66 21 01          lock xacquire and %ax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f0 f3 66 21 01          lock xrelease and %ax,\(%rcx\)
>  [      ]*[a-f0-9]+:    66 f3 89 01             xrelease mov %ax,\(%rcx\)
> +[      ]*[a-f0-9]+:    66 f3 89 04 25 78 56 34 12      xrelease mov %ax,0x12345678
> +[      ]*[a-f0-9]+:    67 66 f3 89 04 25 21 43 65 87   xrelease mov %ax,0x87654321\(,%eiz,1\)
>  [      ]*[a-f0-9]+:    66 f2 f0 09 01          xacquire lock or %ax,\(%rcx\)
>  [      ]*[a-f0-9]+:    66 f2 f0 09 01          xacquire lock or %ax,\(%rcx\)
>  [      ]*[a-f0-9]+:    66 f3 f0 09 01          xrelease lock or %ax,\(%rcx\)
> @@ -526,6 +530,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 21 01             lock xacquire and %eax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f0 f3 21 01             lock xrelease and %eax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f3 89 01                xrelease mov %eax,\(%rcx\)
> +[      ]*[a-f0-9]+:    f3 89 04 25 78 56 34 12         xrelease mov %eax,0x12345678
> +[      ]*[a-f0-9]+:    67 f3 89 04 25 21 43 65 87      xrelease mov %eax,0x87654321\(,%eiz,1\)
>  [      ]*[a-f0-9]+:    f2 f0 09 01             xacquire lock or %eax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f2 f0 09 01             xacquire lock or %eax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f3 f0 09 01             xrelease lock or %eax,\(%rcx\)
> @@ -577,6 +583,8 @@ Disassembly of section .text:
>  [      ]*[a-f0-9]+:    f0 f2 48 21 01          lock xacquire and %rax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f0 f3 48 21 01          lock xrelease and %rax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f3 48 89 01             xrelease mov %rax,\(%rcx\)
> +[      ]*[a-f0-9]+:    f3 48 89 04 25 78 56 34 12      xrelease mov %rax,0x12345678
> +[      ]*[a-f0-9]+:    67 f3 48 89 04 25 21 43 65 87   xrelease mov %rax,0x87654321\(,%eiz,1\)
>  [      ]*[a-f0-9]+:    f2 f0 48 09 01          xacquire lock or %rax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f2 f0 48 09 01          xacquire lock or %rax,\(%rcx\)
>  [      ]*[a-f0-9]+:    f3 f0 48 09 01          xrelease lock or %rax,\(%rcx\)
> --- a/gas/testsuite/gas/i386/x86-64-hle.s
> +++ b/gas/testsuite/gas/i386/x86-64-hle.s
> @@ -442,6 +442,8 @@ _start:
>         .byte 0xf0; .byte 0xf2; andb %al,(%rcx)
>         .byte 0xf0; .byte 0xf3; andb %al,(%rcx)
>         xrelease movb %al,(%rcx)
> +       xrelease movb %al,0x12345678
> +       xrelease addr32 movb %al,0x87654321
>         xacquire lock orb %al,(%rcx)
>         lock xacquire orb %al,(%rcx)
>         xrelease lock orb %al,(%rcx)
> @@ -496,6 +498,8 @@ _start:
>         .byte 0xf0; .byte 0xf2; andw %ax,(%rcx)
>         .byte 0xf0; .byte 0xf3; andw %ax,(%rcx)
>         xrelease movw %ax,(%rcx)
> +       xrelease movw %ax,0x12345678
> +       xrelease addr32 movw %ax,0x87654321
>         xacquire lock orw %ax,(%rcx)
>         lock xacquire orw %ax,(%rcx)
>         xrelease lock orw %ax,(%rcx)
> @@ -550,6 +554,8 @@ _start:
>         .byte 0xf0; .byte 0xf2; andl %eax,(%rcx)
>         .byte 0xf0; .byte 0xf3; andl %eax,(%rcx)
>         xrelease movl %eax,(%rcx)
> +       xrelease movl %eax,0x12345678
> +       xrelease addr32 movl %eax,0x87654321
>         xacquire lock orl %eax,(%rcx)
>         lock xacquire orl %eax,(%rcx)
>         xrelease lock orl %eax,(%rcx)
> @@ -604,6 +610,8 @@ _start:
>         .byte 0xf0; .byte 0xf2; andq %rax,(%rcx)
>         .byte 0xf0; .byte 0xf3; andq %rax,(%rcx)
>         xrelease movq %rax,(%rcx)
> +       xrelease movq %rax,0x12345678
> +       xrelease addr32 movq %rax,0x87654321
>         xacquire lock orq %rax,(%rcx)
>         lock xacquire orq %rax,(%rcx)
>         xrelease lock orq %rax,(%rcx)
>

OK.

Thanks.

-- 
H.J.

  reply	other threads:[~2022-10-11 17:50 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-05  7:19 [PATCH v3 0/7] x86: suffix handling changes Jan Beulich
2022-10-05  7:20 ` [PATCH v3 1/7] x86: constify parse_insn()'s input Jan Beulich
2022-10-05  7:22 ` [PATCH v3 2/7] x86: introduce Pass2 insn attribute Jan Beulich
2022-10-05  7:23 ` [PATCH v3 3/7] x86: re-work insn/suffix recognition Jan Beulich
2022-10-05 23:52   ` H.J. Lu
2022-10-06  6:15     ` Jan Beulich
2022-10-06  6:58       ` Jan Beulich
2022-10-06 15:28         ` H.J. Lu
2022-10-06 16:12           ` Jan Beulich
2022-10-06 18:41             ` H.J. Lu
2022-10-07 13:03               ` Jan Beulich
2022-10-05  7:24 ` [PATCH v3 4/7] x86-64: further re-work insn/suffix recognition to also cover MOVSL Jan Beulich
2022-10-11 17:44   ` H.J. Lu
2022-10-12  7:08     ` Jan Beulich
2022-10-12 17:10       ` H.J. Lu
2022-10-13  6:08         ` Jan Beulich
2022-10-13 17:00           ` H.J. Lu
2022-10-14  7:03             ` Jan Beulich
2022-10-14 17:07               ` H.J. Lu
2022-10-17  7:02                 ` Jan Beulich
2022-10-17 22:36                   ` H.J. Lu
2022-10-18  6:31                     ` Jan Beulich
2022-10-18 21:48                       ` H.J. Lu
2022-10-19  6:08                         ` Jan Beulich
2022-10-19 21:46                           ` H.J. Lu
2022-10-20 10:12                             ` Jan Beulich
2022-10-05  7:24 ` [PATCH v3 5/7] ix86: don't recognize/derive Q suffix in the common case Jan Beulich
2022-10-11 17:49   ` H.J. Lu
2022-10-05  7:25 ` [PATCH v3 6/7] x86-64: allow HLE store of accumulator to absolute 32-bit address Jan Beulich
2022-10-11 17:50   ` H.J. Lu [this message]
2022-10-05  7:25 ` [PATCH v3 7/7] x86: move bad-use-of-TLS-reloc check Jan Beulich
2022-10-11 17:57   ` H.J. Lu
2022-10-12  7:13     ` Jan Beulich
2022-10-12 17:02       ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMe9rOqEf+zmzaxR7brXYWKot9pZgygBL2Hncaw-S+vdf8VjDg@mail.gmail.com \
    --to=hjl.tools@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=jbeulich@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).