[PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[Haochen Jiang]

Hi all,

For all the xh_mode usage in table, they are all using %XH, which will
print "{bad}" while EVEX.W=1. This makes this vex.w check unnecessary.

Another reason for removing this check is because xh_mode with
broadcast usage should not indicate the vex.w bit. Instead it should
be handled by a W table entry.

BRs,
Haochen

opcodes/ChangeLog:

	* i386-dis.c (OP_E_memory): Remove vex.w check for xh_mode.
---
 opcodes/i386-dis.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index e43666af84..e778e91920 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -12206,24 +12206,19 @@ OP_E_memory (instr_info *ins, int bytemode, int sizeflag)
 	{
 	  if (bytemode == xh_mode)
 	    {
-	      if (ins->vex.w)
-		oappend (ins, "{bad}");
-	      else
+	      switch (ins->vex.length)
 		{
-		  switch (ins->vex.length)
-		    {
-		    case 128:
-		      oappend (ins, "{1to8}");
-		      break;
-		    case 256:
-		      oappend (ins, "{1to16}");
-		      break;
-		    case 512:
-		      oappend (ins, "{1to32}");
-		      break;
-		    default:
-		      abort ();
-		    }
+		case 128:
+		  oappend (ins, "{1to8}");
+		  break;
+		case 256:
+		  oappend (ins, "{1to16}");
+		  break;
+		case 512:
+		  oappend (ins, "{1to32}");
+		  break;
+		default:
+		  abort ();
 		}
 	    }
 	  else if (bytemode == q_mode
-- 
2.18.1

Re: [PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[Jan Beulich]

On 05.12.2022 03:53, Haochen Jiang wrote:
> For all the xh_mode usage in table, they are all using %XH, which will
> print "{bad}" while EVEX.W=1. This makes this vex.w check unnecessary.

I agree with this part.

> Another reason for removing this check is because xh_mode with
> broadcast usage should not indicate the vex.w bit. Instead it should
> be handled by a W table entry.

There are no broadcast insns using xh_mode, and you can't really mean
embedded broadcast (because EVEX.W needs to be properly encoded there).
Hence I don't follow what you're talking about here.

The code change is okay, but if the above is to become the commit
message, the raised question will need addressing first.

Jan

RE: [PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[Jiang, Haochen]

> > Another reason for removing this check is because xh_mode with
> > broadcast usage should not indicate the vex.w bit. Instead it should
> > be handled by a W table entry.
> 
> There are no broadcast insns using xh_mode, and you can't really mean
> embedded broadcast (because EVEX.W needs to be properly encoded
> there).
> Hence I don't follow what you're talking about here.

Maybe this part is a little misleading. I will delete that in commit message.
What I actually mean is just like what you commented.

I suppose using a W table entry in the future to encode here if not using
something like %XH.

Haochen

> 
> The code change is okay, but if the above is to become the commit
> message, the raised question will need addressing first.
> 
> Jan

Re: [PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[H.J. Lu]

On Mon, Dec 5, 2022 at 5:36 PM Jiang, Haochen <haochen.jiang@intel.com> wrote:
>
> > > Another reason for removing this check is because xh_mode with
> > > broadcast usage should not indicate the vex.w bit. Instead it should
> > > be handled by a W table entry.
> >
> > There are no broadcast insns using xh_mode, and you can't really mean
> > embedded broadcast (because EVEX.W needs to be properly encoded
> > there).
> > Hence I don't follow what you're talking about here.
>
> Maybe this part is a little misleading. I will delete that in commit message.
> What I actually mean is just like what you commented.
>
> I suppose using a W table entry in the future to encode here if not using
> something like %XH.

If vex.w == 1 check is removed, disassembler will display the wrong instruction.

> Haochen
>
> >
> > The code change is okay, but if the above is to become the commit
> > message, the raised question will need addressing first.
> >
> > Jan



-- 
H.J.

RE: [PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[Jiang, Haochen]

> -----Original Message-----
> From: H.J. Lu <hjl.tools@gmail.com>
> Sent: Tuesday, December 6, 2022 11:40 AM
> To: Jiang, Haochen <haochen.jiang@intel.com>
> Cc: Beulich, Jan <JBeulich@suse.com>; binutils@sourceware.org
> Subject: Re: [PATCH] x86: Remove unnecessary vex.w check for xh_mode in
> disassembler
> 
> On Mon, Dec 5, 2022 at 5:36 PM Jiang, Haochen <haochen.jiang@intel.com>
> wrote:
> >
> > > > Another reason for removing this check is because xh_mode with
> > > > broadcast usage should not indicate the vex.w bit. Instead it
> > > > should be handled by a W table entry.
> > >
> > > There are no broadcast insns using xh_mode, and you can't really
> > > mean embedded broadcast (because EVEX.W needs to be properly
> encoded
> > > there).
> > > Hence I don't follow what you're talking about here.
> >
> > Maybe this part is a little misleading. I will delete that in commit message.
> > What I actually mean is just like what you commented.
> >
> > I suppose using a W table entry in the future to encode here if not
> > using something like %XH.
> 
> If vex.w == 1 check is removed, disassembler will display the wrong
> instruction.

No, it will still print "{bad}" for all instructions currently since all of them are
using %XH, which will do this thing.

Maybe you are concerned about future instruction behavior w/o using %XH?

Haochen

> 
> > Haochen
> >
> > >
> > > The code change is okay, but if the above is to become the commit
> > > message, the raised question will need addressing first.
> > >
> > > Jan
> 
> 
> 
> --
> H.J.

Re: [PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[Jan Beulich]

On 06.12.2022 02:36, Jiang, Haochen wrote:
>>> Another reason for removing this check is because xh_mode with
>>> broadcast usage should not indicate the vex.w bit. Instead it should
>>> be handled by a W table entry.
>>
>> There are no broadcast insns using xh_mode, and you can't really mean
>> embedded broadcast (because EVEX.W needs to be properly encoded
>> there).
>> Hence I don't follow what you're talking about here.
> 
> Maybe this part is a little misleading. I will delete that in commit message.
> What I actually mean is just like what you commented.
> 
> I suppose using a W table entry in the future to encode here if not using
> something like %XH.

Feel free to commit with that text dropped; aiui you've addressed H.J.'s
question.

Jan

RE: [PATCH] x86: Remove unnecessary vex.w check for xh_mode in disassembler

[Jiang, Haochen]

> On 06.12.2022 02:36, Jiang, Haochen wrote:
> >>> Another reason for removing this check is because xh_mode with
> >>> broadcast usage should not indicate the vex.w bit. Instead it should
> >>> be handled by a W table entry.
> >>
> >> There are no broadcast insns using xh_mode, and you can't really mean
> >> embedded broadcast (because EVEX.W needs to be properly encoded
> >> there).
> >> Hence I don't follow what you're talking about here.
> >
> > Maybe this part is a little misleading. I will delete that in commit message.
> > What I actually mean is just like what you commented.
> >
> > I suppose using a W table entry in the future to encode here if not
> > using something like %XH.
> 
> Feel free to commit with that text dropped; aiui you've addressed H.J.'s
> question.

Ok. I will commit the patch with that text dropped.

Thanks for your review!

Haochen

> 
> Jan