* [PATCH] x86: Rename VexOpcode to OpcodePrefix
@ 2020-10-13 21:49 H.J. Lu
2020-10-14 2:21 ` V2 " H.J. Lu
0 siblings, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2020-10-13 21:49 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 545 bytes --]
Rename VexOpcode to OpcodePrefix so that OpcodePrefix can be used for
regular encoding prefix.
gas/
* config/tc-i386.c (build_vex_prefix): Replace vexopcode with
opcodeprefix.
(build_evex_prefix): Likewise.
(is_any_vex_encoding): Don't check vexopcode.
(output_insn): Handle opcodeprefix.
opcodes/
* i386-gen.c (opcode_modifiers): Replace VexOpcode with
OpcodePrefix.
* i386-opc.h (VexOpcode): Renamed to ...
(OpcodePrefix): This.
(PREFIX_NONE): New.
(PREFIX_F3): Likewise.
* i386-opc.tbl: Replace VexOpcode= with OpcodePrefix=.
[-- Attachment #2: 0001-x86-Rename-VexOpcode-to-OpcodePrefix.patch.xz --]
[-- Type: application/x-xz, Size: 35404 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix
2020-10-13 21:49 [PATCH] x86: Rename VexOpcode to OpcodePrefix H.J. Lu
@ 2020-10-14 2:21 ` H.J. Lu
2020-10-14 2:25 ` [PATCH] x86: Remove the prefix byte from base_opcode H.J. Lu
2020-10-14 12:38 ` V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix Jan Beulich
0 siblings, 2 replies; 8+ messages in thread
From: H.J. Lu @ 2020-10-14 2:21 UTC (permalink / raw)
To: Binutils
[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]
On Tue, Oct 13, 2020 at 2:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Rename VexOpcode to OpcodePrefix so that OpcodePrefix can be used for
> regular encoding prefix.
>
> gas/
>
> * config/tc-i386.c (build_vex_prefix): Replace vexopcode with
> opcodeprefix.
> (build_evex_prefix): Likewise.
> (is_any_vex_encoding): Don't check vexopcode.
> (output_insn): Handle opcodeprefix.
>
> opcodes/
>
> * i386-gen.c (opcode_modifiers): Replace VexOpcode with
> OpcodePrefix.
> * i386-opc.h (VexOpcode): Renamed to ...
> (OpcodePrefix): This.
> (PREFIX_NONE): New.
> (PREFIX_F3): Likewise.
> * i386-opc.tbl: Replace VexOpcode= with OpcodePrefix=.
This is the patch I am checking in.
gas/
* config/tc-i386.c (build_vex_prefix): Replace vexopcode with
opcodeprefix.
(build_evex_prefix): Likewise.
(is_any_vex_encoding): Don't check vexopcode.
(output_insn): Handle opcodeprefix.
opcodes/
* i386-gen.c (opcode_modifiers): Replace VexOpcode with
OpcodePrefix.
* i386-opc.h (VexOpcode): Renamed to ...
(OpcodePrefix): This.
(PREFIX_NONE): New.
(PREFIX_0X66): Likewise.
(PREFIX_0XF2): Likewise.
(PREFIX_0XF3): Likewise.
* i386-opc.tbl (Prefix_0X66): New.
(Prefix_0XF2): Likewise.
(Prefix_0XF3): Likewise.
Replace VexOpcode= with OpcodePrefix=. Use Prefix_0X66 on xorpd.
Use Prefix_0XF3 on cvtdq2pd. Use Prefix_0XF2 on cvtpd2dq.
* i386-tbl.h: Regenerated.
--
H.J.
[-- Attachment #2: 0001-x86-Rename-VexOpcode-to-OpcodePrefix.patch.xz --]
[-- Type: application/x-xz, Size: 35880 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] x86: Remove the prefix byte from base_opcode
2020-10-14 2:21 ` V2 " H.J. Lu
@ 2020-10-14 2:25 ` H.J. Lu
2020-10-14 4:38 ` [PATCH] x86: Remove the prefix byte from non-VEX/EVEX base_opcode H.J. Lu
2020-10-14 12:38 ` V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix Jan Beulich
1 sibling, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2020-10-14 2:25 UTC (permalink / raw)
To: Binutils
[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]
On Tue, Oct 13, 2020 at 7:21 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Oct 13, 2020 at 2:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > Rename VexOpcode to OpcodePrefix so that OpcodePrefix can be used for
> > regular encoding prefix.
> >
> > gas/
> >
> > * config/tc-i386.c (build_vex_prefix): Replace vexopcode with
> > opcodeprefix.
> > (build_evex_prefix): Likewise.
> > (is_any_vex_encoding): Don't check vexopcode.
> > (output_insn): Handle opcodeprefix.
> >
> > opcodes/
> >
> > * i386-gen.c (opcode_modifiers): Replace VexOpcode with
> > OpcodePrefix.
> > * i386-opc.h (VexOpcode): Renamed to ...
> > (OpcodePrefix): This.
> > (PREFIX_NONE): New.
> > (PREFIX_F3): Likewise.
> > * i386-opc.tbl: Replace VexOpcode= with OpcodePrefix=.
>
> This is the patch I am checking in.
>
> gas/
>
> * config/tc-i386.c (build_vex_prefix): Replace vexopcode with
> opcodeprefix.
> (build_evex_prefix): Likewise.
> (is_any_vex_encoding): Don't check vexopcode.
> (output_insn): Handle opcodeprefix.
>
> opcodes/
>
> * i386-gen.c (opcode_modifiers): Replace VexOpcode with
> OpcodePrefix.
> * i386-opc.h (VexOpcode): Renamed to ...
> (OpcodePrefix): This.
> (PREFIX_NONE): New.
> (PREFIX_0X66): Likewise.
> (PREFIX_0XF2): Likewise.
> (PREFIX_0XF3): Likewise.
> * i386-opc.tbl (Prefix_0X66): New.
> (Prefix_0XF2): Likewise.
> (Prefix_0XF3): Likewise.
> Replace VexOpcode= with OpcodePrefix=. Use Prefix_0X66 on xorpd.
> Use Prefix_0XF3 on cvtdq2pd. Use Prefix_0XF2 on cvtpd2dq.
> * i386-tbl.h: Regenerated.
>
Here is the followup patch to remove the prefix byte from
base_opcode.
Replace the prefix byte in base_opcode with PREFIX_0X66, PREFIX_0XF2 or
PREFIX_0XF3.
gas/
* config/tc-i386.c (load_insn_p): Check opcodeprefix == 0 for
base_opcode == 0xfc7.
(match_template): Likewise.
(process_suffix): Check opcodeprefix == PREFIX_0XF2 for CRC32.
(check_byte_reg): Likewise.
(output_insn): Abort if base_opcode has a prefix byte.
opcodes/
* i386-opc.tbl: Replace the prefix byte in base_opcode with
PREFIX_0X66, PREFIX_0XF2 or PREFIX_0XF3.
* i386-tbl.h: Regenerated.
--
H.J.
[-- Attachment #2: 0001-x86-Remove-the-prefix-byte-from-base_opcode.patch.xz --]
[-- Type: application/x-xz, Size: 22384 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] x86: Remove the prefix byte from non-VEX/EVEX base_opcode
2020-10-14 2:25 ` [PATCH] x86: Remove the prefix byte from base_opcode H.J. Lu
@ 2020-10-14 4:38 ` H.J. Lu
0 siblings, 0 replies; 8+ messages in thread
From: H.J. Lu @ 2020-10-14 4:38 UTC (permalink / raw)
To: Binutils
[-- Attachment #1: Type: text/plain, Size: 2473 bytes --]
On Tue, Oct 13, 2020 at 7:25 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Oct 13, 2020 at 7:21 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Oct 13, 2020 at 2:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > Rename VexOpcode to OpcodePrefix so that OpcodePrefix can be used for
> > > regular encoding prefix.
> > >
> > > gas/
> > >
> > > * config/tc-i386.c (build_vex_prefix): Replace vexopcode with
> > > opcodeprefix.
> > > (build_evex_prefix): Likewise.
> > > (is_any_vex_encoding): Don't check vexopcode.
> > > (output_insn): Handle opcodeprefix.
> > >
> > > opcodes/
> > >
> > > * i386-gen.c (opcode_modifiers): Replace VexOpcode with
> > > OpcodePrefix.
> > > * i386-opc.h (VexOpcode): Renamed to ...
> > > (OpcodePrefix): This.
> > > (PREFIX_NONE): New.
> > > (PREFIX_F3): Likewise.
> > > * i386-opc.tbl: Replace VexOpcode= with OpcodePrefix=.
> >
> > This is the patch I am checking in.
> >
> > gas/
> >
> > * config/tc-i386.c (build_vex_prefix): Replace vexopcode with
> > opcodeprefix.
> > (build_evex_prefix): Likewise.
> > (is_any_vex_encoding): Don't check vexopcode.
> > (output_insn): Handle opcodeprefix.
> >
> > opcodes/
> >
> > * i386-gen.c (opcode_modifiers): Replace VexOpcode with
> > OpcodePrefix.
> > * i386-opc.h (VexOpcode): Renamed to ...
> > (OpcodePrefix): This.
> > (PREFIX_NONE): New.
> > (PREFIX_0X66): Likewise.
> > (PREFIX_0XF2): Likewise.
> > (PREFIX_0XF3): Likewise.
> > * i386-opc.tbl (Prefix_0X66): New.
> > (Prefix_0XF2): Likewise.
> > (Prefix_0XF3): Likewise.
> > Replace VexOpcode= with OpcodePrefix=. Use Prefix_0X66 on xorpd.
> > Use Prefix_0XF3 on cvtdq2pd. Use Prefix_0XF2 on cvtpd2dq.
> > * i386-tbl.h: Regenerated.
> >
>
> Here is the followup patch to remove the prefix byte from
> base_opcode.
>
> Replace the prefix byte in base_opcode with PREFIX_0X66, PREFIX_0XF2 or
> PREFIX_0XF3.
>
> gas/
>
> * config/tc-i386.c (load_insn_p): Check opcodeprefix == 0 for
> base_opcode == 0xfc7.
> (match_template): Likewise.
> (process_suffix): Check opcodeprefix == PREFIX_0XF2 for CRC32.
> (check_byte_reg): Likewise.
> (output_insn): Abort if base_opcode has a prefix byte.
>
> opcodes/
>
> * i386-opc.tbl: Replace the prefix byte in base_opcode with
> PREFIX_0X66, PREFIX_0XF2 or PREFIX_0XF3.
> * i386-tbl.h: Regenerated.
>
The updated patch to move the non-VEX/EVEX base_opcode check
to i386-gen.c.
--
H.J.
[-- Attachment #2: 0001-x86-Remove-the-prefix-byte-from-non-VEX-EVEX-base_op.patch.xz --]
[-- Type: application/x-xz, Size: 23112 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix
2020-10-14 2:21 ` V2 " H.J. Lu
2020-10-14 2:25 ` [PATCH] x86: Remove the prefix byte from base_opcode H.J. Lu
@ 2020-10-14 12:38 ` Jan Beulich
2020-10-14 12:55 ` H.J. Lu
1 sibling, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2020-10-14 12:38 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On 14.10.2020 04:21, H.J. Lu via Binutils wrote:
> This is the patch I am checking in.
>
> gas/
>
> * config/tc-i386.c (build_vex_prefix): Replace vexopcode with
> opcodeprefix.
> (build_evex_prefix): Likewise.
> (is_any_vex_encoding): Don't check vexopcode.
> (output_insn): Handle opcodeprefix.
>
> opcodes/
>
> * i386-gen.c (opcode_modifiers): Replace VexOpcode with
> OpcodePrefix.
> * i386-opc.h (VexOpcode): Renamed to ...
> (OpcodePrefix): This.
> (PREFIX_NONE): New.
> (PREFIX_0X66): Likewise.
> (PREFIX_0XF2): Likewise.
> (PREFIX_0XF3): Likewise.
Why 0X66 etc and not just 66?
> * i386-opc.tbl (Prefix_0X66): New.
> (Prefix_0XF2): Likewise.
> (Prefix_0XF3): Likewise.
> Replace VexOpcode= with OpcodePrefix=. Use Prefix_0X66 on xorpd.
> Use Prefix_0XF3 on cvtdq2pd. Use Prefix_0XF2 on cvtpd2dq.
Is this an arbitrary choice? Such things would be helpful to be
spelled out in a little bit of a description, not just for reviewing
now, but also when later running into this commit. If so, I expect
there'll be a full run through all opcodes to replace the current
model with the new one?
I'd also like to note that your "Use prefix ... on ..." isn't
actually correct, as you only changed the non-SSE2AVX encodings.
(quoting the actual patch)
>--- a/opcodes/i386-opc.h
>+++ b/opcodes/i386-opc.h
>@@ -561,6 +561,16 @@ enum
> #define VEXW1 2
> #define VEXWIG 3
> VexW,
>+ /* Regular opcode prefix:
>+ 0: None
>+ 1: Add 0x66 opcode prefix.
>+ 2: Add 0xf2 opcode prefix.
>+ 3: Add 0xf3 opcode prefix.
>+ */
>+#define PREFIX_NONE 0
>+#define PREFIX_0X66 1
>+#define PREFIX_0XF2 2
>+#define PREFIX_0XF3 3
> /* VEX opcode prefix:
> 0: VEX 0x0F opcode prefix.
> 1: VEX 0x0F38 opcode prefix.
>@@ -575,7 +585,7 @@ enum
> #define XOP08 3
> #define XOP09 4
> #define XOP0A 5
>- VexOpcode,
>+ OpcodePrefix,
I don't think this is a helpful repurposing. Instead I was
thinking to encode 0F, 0F38, and 0F3A for non-VEX the same way,
re-using the encodings here. 66, F3, and F2 as a result would
need separate encoding.
I also think it would have been more logical (because of the
way VEX/XOP/EVEX encode them) if it were
#define PREFIX_NONE 0
#define PREFIX_66 1
#define PREFIX_F3 2
#define PREFIX_F2 3
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix
2020-10-14 12:38 ` V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix Jan Beulich
@ 2020-10-14 12:55 ` H.J. Lu
2020-10-14 13:18 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2020-10-14 12:55 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Wed, Oct 14, 2020 at 5:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.10.2020 04:21, H.J. Lu via Binutils wrote:
> > This is the patch I am checking in.
> >
> > gas/
> >
> > * config/tc-i386.c (build_vex_prefix): Replace vexopcode with
> > opcodeprefix.
> > (build_evex_prefix): Likewise.
> > (is_any_vex_encoding): Don't check vexopcode.
> > (output_insn): Handle opcodeprefix.
> >
> > opcodes/
> >
> > * i386-gen.c (opcode_modifiers): Replace VexOpcode with
> > OpcodePrefix.
> > * i386-opc.h (VexOpcode): Renamed to ...
> > (OpcodePrefix): This.
> > (PREFIX_NONE): New.
> > (PREFIX_0X66): Likewise.
> > (PREFIX_0XF2): Likewise.
> > (PREFIX_0XF3): Likewise.
>
> Why 0X66 etc and not just 66?
Free feel to remove 0X.
> > * i386-opc.tbl (Prefix_0X66): New.
> > (Prefix_0XF2): Likewise.
> > (Prefix_0XF3): Likewise.
> > Replace VexOpcode= with OpcodePrefix=. Use Prefix_0X66 on xorpd.
> > Use Prefix_0XF3 on cvtdq2pd. Use Prefix_0XF2 on cvtpd2dq.
>
> Is this an arbitrary choice? Such things would be helpful to be
> spelled out in a little bit of a description, not just for reviewing
How about
// Add 0x66/0xf2/0xf3 prefix to non-VEX/EVEX/prefix instructions.
#define Prefix_0X66 OpcodePrefix=PREFIX_0X66
#define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
#define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
> now, but also when later running into this commit. If so, I expect
> there'll be a full run through all opcodes to replace the current
> model with the new one?
This is
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8b65b8953af2d49ae1d2d7fcc5b49c5308febbc1
> I'd also like to note that your "Use prefix ... on ..." isn't
> actually correct, as you only changed the non-SSE2AVX encodings.
It should be non-VEX/EVEX/prefix instructions.
> (quoting the actual patch)
>
> >--- a/opcodes/i386-opc.h
> >+++ b/opcodes/i386-opc.h
> >@@ -561,6 +561,16 @@ enum
> > #define VEXW1 2
> > #define VEXWIG 3
> > VexW,
> >+ /* Regular opcode prefix:
> >+ 0: None
> >+ 1: Add 0x66 opcode prefix.
> >+ 2: Add 0xf2 opcode prefix.
> >+ 3: Add 0xf3 opcode prefix.
> >+ */
> >+#define PREFIX_NONE 0
> >+#define PREFIX_0X66 1
> >+#define PREFIX_0XF2 2
> >+#define PREFIX_0XF3 3
> > /* VEX opcode prefix:
> > 0: VEX 0x0F opcode prefix.
> > 1: VEX 0x0F38 opcode prefix.
> >@@ -575,7 +585,7 @@ enum
> > #define XOP08 3
> > #define XOP09 4
> > #define XOP0A 5
> >- VexOpcode,
> >+ OpcodePrefix,
>
> I don't think this is a helpful repurposing. Instead I was
> thinking to encode 0F, 0F38, and 0F3A for non-VEX the same way,
> re-using the encodings here. 66, F3, and F2 as a result would
> need separate encoding.
Free feel to change. It should be easy to change with:
#define Prefix_0X66 OpcodePrefix=PREFIX_0X66
#define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
#define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
> I also think it would have been more logical (because of the
> way VEX/XOP/EVEX encode them) if it were
>
> #define PREFIX_NONE 0
> #define PREFIX_66 1
> #define PREFIX_F3 2
> #define PREFIX_F2 3
>
Can you submit a patch?
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix
2020-10-14 12:55 ` H.J. Lu
@ 2020-10-14 13:18 ` Jan Beulich
2020-10-14 13:29 ` H.J. Lu
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2020-10-14 13:18 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On 14.10.2020 14:55, H.J. Lu wrote:
> On Wed, Oct 14, 2020 at 5:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 14.10.2020 04:21, H.J. Lu via Binutils wrote:
>>> * i386-opc.tbl (Prefix_0X66): New.
>>> (Prefix_0XF2): Likewise.
>>> (Prefix_0XF3): Likewise.
>>> Replace VexOpcode= with OpcodePrefix=. Use Prefix_0X66 on xorpd.
>>> Use Prefix_0XF3 on cvtdq2pd. Use Prefix_0XF2 on cvtpd2dq.
>>
>> Is this an arbitrary choice? Such things would be helpful to be
>> spelled out in a little bit of a description, not just for reviewing
>
> How about
>
> // Add 0x66/0xf2/0xf3 prefix to non-VEX/EVEX/prefix instructions.
> #define Prefix_0X66 OpcodePrefix=PREFIX_0X66
> #define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
> #define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
I don't understand your reply, which makes me assume you didn't
understand my question. I was asking whether the choice of
adjusting just xorpd, cvtdq2pd, and cvtpd2dq was arbitrary.
>> now, but also when later running into this commit. If so, I expect
>> there'll be a full run through all opcodes to replace the current
>> model with the new one?
>
> This is
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8b65b8953af2d49ae1d2d7fcc5b49c5308febbc1
Yes, I've seen this in the meantime, thanks. The same will of
course want doing for the VEX etc encoded insns.
>> I'd also like to note that your "Use prefix ... on ..." isn't
>> actually correct, as you only changed the non-SSE2AVX encodings.
>
> It should be non-VEX/EVEX/prefix instructions.
>
>> (quoting the actual patch)
>>
>>> --- a/opcodes/i386-opc.h
>>> +++ b/opcodes/i386-opc.h
>>> @@ -561,6 +561,16 @@ enum
>>> #define VEXW1 2
>>> #define VEXWIG 3
>>> VexW,
>>> + /* Regular opcode prefix:
>>> + 0: None
>>> + 1: Add 0x66 opcode prefix.
>>> + 2: Add 0xf2 opcode prefix.
>>> + 3: Add 0xf3 opcode prefix.
>>> + */
>>> +#define PREFIX_NONE 0
>>> +#define PREFIX_0X66 1
>>> +#define PREFIX_0XF2 2
>>> +#define PREFIX_0XF3 3
>>> /* VEX opcode prefix:
>>> 0: VEX 0x0F opcode prefix.
>>> 1: VEX 0x0F38 opcode prefix.
>>> @@ -575,7 +585,7 @@ enum
>>> #define XOP08 3
>>> #define XOP09 4
>>> #define XOP0A 5
>>> - VexOpcode,
>>> + OpcodePrefix,
>>
>> I don't think this is a helpful repurposing. Instead I was
>> thinking to encode 0F, 0F38, and 0F3A for non-VEX the same way,
>> re-using the encodings here. 66, F3, and F2 as a result would
>> need separate encoding.
>
> Free feel to change. It should be easy to change with:
>
> #define Prefix_0X66 OpcodePrefix=PREFIX_0X66
> #define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
> #define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
>
>> I also think it would have been more logical (because of the
>> way VEX/XOP/EVEX encode them) if it were
>>
>> #define PREFIX_NONE 0
>> #define PREFIX_66 1
>> #define PREFIX_F3 2
>> #define PREFIX_F2 3
>
> Can you submit a patch?
Sure. I may be able to find time for this in several months ...
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix
2020-10-14 13:18 ` Jan Beulich
@ 2020-10-14 13:29 ` H.J. Lu
0 siblings, 0 replies; 8+ messages in thread
From: H.J. Lu @ 2020-10-14 13:29 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Wed, Oct 14, 2020 at 6:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.10.2020 14:55, H.J. Lu wrote:
> > On Wed, Oct 14, 2020 at 5:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 14.10.2020 04:21, H.J. Lu via Binutils wrote:
> >>> * i386-opc.tbl (Prefix_0X66): New.
> >>> (Prefix_0XF2): Likewise.
> >>> (Prefix_0XF3): Likewise.
> >>> Replace VexOpcode= with OpcodePrefix=. Use Prefix_0X66 on xorpd.
> >>> Use Prefix_0XF3 on cvtdq2pd. Use Prefix_0XF2 on cvtpd2dq.
> >>
> >> Is this an arbitrary choice? Such things would be helpful to be
> >> spelled out in a little bit of a description, not just for reviewing
> >
> > How about
> >
> > // Add 0x66/0xf2/0xf3 prefix to non-VEX/EVEX/prefix instructions.
> > #define Prefix_0X66 OpcodePrefix=PREFIX_0X66
> > #define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
> > #define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
>
> I don't understand your reply, which makes me assume you didn't
> understand my question. I was asking whether the choice of
> adjusting just xorpd, cvtdq2pd, and cvtpd2dq was arbitrary.
It was a trial run, followed by the full patch.
> >> now, but also when later running into this commit. If so, I expect
> >> there'll be a full run through all opcodes to replace the current
> >> model with the new one?
> >
> > This is
> >
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8b65b8953af2d49ae1d2d7fcc5b49c5308febbc1
>
> Yes, I've seen this in the meantime, thanks. The same will of
> course want doing for the VEX etc encoded insns.
>
> >> I'd also like to note that your "Use prefix ... on ..." isn't
> >> actually correct, as you only changed the non-SSE2AVX encodings.
> >
> > It should be non-VEX/EVEX/prefix instructions.
> >
> >> (quoting the actual patch)
> >>
> >>> --- a/opcodes/i386-opc.h
> >>> +++ b/opcodes/i386-opc.h
> >>> @@ -561,6 +561,16 @@ enum
> >>> #define VEXW1 2
> >>> #define VEXWIG 3
> >>> VexW,
> >>> + /* Regular opcode prefix:
> >>> + 0: None
> >>> + 1: Add 0x66 opcode prefix.
> >>> + 2: Add 0xf2 opcode prefix.
> >>> + 3: Add 0xf3 opcode prefix.
> >>> + */
> >>> +#define PREFIX_NONE 0
> >>> +#define PREFIX_0X66 1
> >>> +#define PREFIX_0XF2 2
> >>> +#define PREFIX_0XF3 3
> >>> /* VEX opcode prefix:
> >>> 0: VEX 0x0F opcode prefix.
> >>> 1: VEX 0x0F38 opcode prefix.
> >>> @@ -575,7 +585,7 @@ enum
> >>> #define XOP08 3
> >>> #define XOP09 4
> >>> #define XOP0A 5
> >>> - VexOpcode,
> >>> + OpcodePrefix,
> >>
> >> I don't think this is a helpful repurposing. Instead I was
> >> thinking to encode 0F, 0F38, and 0F3A for non-VEX the same way,
> >> re-using the encodings here. 66, F3, and F2 as a result would
> >> need separate encoding.
> >
> > Free feel to change. It should be easy to change with:
> >
> > #define Prefix_0X66 OpcodePrefix=PREFIX_0X66
> > #define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
> > #define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
> >
> >> I also think it would have been more logical (because of the
> >> way VEX/XOP/EVEX encode them) if it were
> >>
> >> #define PREFIX_NONE 0
> >> #define PREFIX_66 1
> >> #define PREFIX_F3 2
> >> #define PREFIX_F2 3
> >
> > Can you submit a patch?
>
> Sure. I may be able to find time for this in several months ...
>
> Jan
--
H.J.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-14 13:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13 21:49 [PATCH] x86: Rename VexOpcode to OpcodePrefix H.J. Lu
2020-10-14 2:21 ` V2 " H.J. Lu
2020-10-14 2:25 ` [PATCH] x86: Remove the prefix byte from base_opcode H.J. Lu
2020-10-14 4:38 ` [PATCH] x86: Remove the prefix byte from non-VEX/EVEX base_opcode H.J. Lu
2020-10-14 12:38 ` V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix Jan Beulich
2020-10-14 12:55 ` H.J. Lu
2020-10-14 13:18 ` Jan Beulich
2020-10-14 13:29 ` H.J. Lu
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).