diff mbox series

x86: SETcc doesn't permit W suffix

Message ID 576446e9-6ebe-4911-b9f6-df4338e93565@suse.com
State New
Headers
Series x86: SETcc doesn't permit W suffix |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich Nov. 18, 2024, 10:50 a.m. UTC 
  Accidentally I had removed No_wSuf when cloning the extra template.

Comments

Jan Beulich Nov. 18, 2024, 11:04 a.m. UTC | #1 
On 18.11.2024 11:50, Jan Beulich wrote:
> Accidentally I had removed No_wSuf when cloning the extra template.
> 
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -562,7 +562,7 @@ set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|N
>  setzu<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|VexWIG|EVexMap4|ZU, { Reg8 }
>  // As an extension, allow SETcc without the ZU infix, instead taking the full
>  // register as operand.
> -set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_bSuf|No_sSuf|EVexMap4|ZU, { Reg32|Reg64 }
> +set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_bSuf|No_wSuf|No_sSuf|EVexMap4|ZU, { Reg32|Reg64 }

While I noticed the above issue in the context of "x86/Solaris: support
Sun form of CMOVcc", I'll arbitrarily reply here: That same documentation
(as before, the only AT&T spec I'm aware of) does specifically not name
'b' suffixes for SETcc. Still we allow them, and we even have one single
test for it. It feels wrong though. Probably we shouldn't remove support
for the pre-APX form. Yet I'm heavily inclined to disallow it for the APX
ones that we gained only relatively recently. Thoughts?

Jan
Cui, Lili Nov. 21, 2024, 6:31 a.m. UTC | #2 
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, November 18, 2024 7:04 PM
> To: H.J. Lu <hjl.tools@gmail.com>; Cui, Lili <lili.cui@intel.com>
> Cc: Binutils <binutils@sourceware.org>
> Subject: Re: [PATCH] x86: SETcc doesn't permit W suffix
> 
> On 18.11.2024 11:50, Jan Beulich wrote:
> > Accidentally I had removed No_wSuf when cloning the extra template.
> >
> > --- a/opcodes/i386-opc.tbl
> > +++ b/opcodes/i386-opc.tbl
> > @@ -562,7 +562,7 @@ set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|N
> > setzu<cc>, 0xf24<cc:opc>/0, APX_F,
> > Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|VexWIG|EVexMap4|ZU,
> { Reg8 }  //
> > As an extension, allow SETcc without the ZU infix, instead taking the full  //
> register as operand.
> > -set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_bSuf|No_sSuf|EVexMap4|ZU,
> {
> > Reg32|Reg64 }
> > +set<cc>, 0xf24<cc:opc>/0, APX_F,
> > +Modrm|No_bSuf|No_wSuf|No_sSuf|EVexMap4|ZU, { Reg32|Reg64 }
> 
> While I noticed the above issue in the context of "x86/Solaris: support Sun
> form of CMOVcc", I'll arbitrarily reply here: That same documentation (as
> before, the only AT&T spec I'm aware of) does specifically not name 'b' suffixes
> for SETcc. Still we allow them, and we even have one single test for it. It feels
> wrong though. Probably we shouldn't remove support for the pre-APX form.
> Yet I'm heavily inclined to disallow it for the APX ones that we gained only
> relatively recently. Thoughts? Either both prohibit b, or both allow b.

It looks like SDM doesn't disable the b suffix for SETcc, but I see your point, our default output is b now, I think it's better to keep them the same, either disabling b in both or allowing b in both.

Thanks,
Lili.
diff mbox series

Patch

--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -562,7 +562,7 @@  set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|N
 setzu<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|VexWIG|EVexMap4|ZU, { Reg8 }
 // As an extension, allow SETcc without the ZU infix, instead taking the full
 // register as operand.
-set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_bSuf|No_sSuf|EVexMap4|ZU, { Reg32|Reg64 }
+set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_bSuf|No_wSuf|No_sSuf|EVexMap4|ZU, { Reg32|Reg64 }
 
 // String manipulation.
 cmps, 0xa6, 0, W|No_sSuf|RepPrefixOk|IntelSuffix, {}