RE: FW: [PATCH 3/8] Add tests for APX GPR32 with extend evex prefix

["Cui, Lili" <lili.cui@intel.com>]

> On 18.10.2023 04:32, Cui, Lili wrote:
> >>>>> --- a/gas/testsuite/gas/i386/x86-64-inval-movbe.s
> >>>>> +++ b/gas/testsuite/gas/i386/x86-64-inval-movbe.s
> >>>>> @@ -1,5 +1,6 @@
> >>>>>  # Check illegal movbe in 64bit mode.
> >>>>>  	.text
> >>>>> +	.arch .noapx_f
> >>>>>  foo:
> >>>>>  	movbe	(%rcx),%bl
> >>>>>  	movbe	%ecx,%ebx
> >>>>
> >>>> I don't understand the need for this addition (and hence for the
> >>>> need to change the test's expecations). Like was mentioned on the
> >>>> original
> >>>> AVX10 series, tests like this shall not need modification, or else
> >>>> it indicates people's code also may need ".arch .noapx_f"
> >>>> additions, which I'm sure you agree may not be required. Finally,
> >>>> if testcase expecations like the above would be needed anywhere,
> >>>> please generalize them such that a similar mere addition of a line
> >>>> doesn't require the entire test to be touched. Here this means that
> >>>> while for the diagnostics you of course want exact line number
> >>>> matches, for the actual listing line numbers don't don't need matching
> individually.
> >>>>
> >>>
> >>> Agree with you, but movbe is special, movbe didn't support reg to
> >>> reg
> >> before, but APX enable it. so I added .arch .noapx_f for this invalid test.
> >>
> >> To be honest I didn't really notice this difference so far. That's
> >> somewhat ugly, because people mistakenly using a reg->reg form would
> >> suddenly get an EVEX encoding rather than an error. This will need
> >> some further thought; please mention the "anomaly" explicitly in the
> description of the respective patch.
> >>
> > SPEC:
> > Note: The promoted versions of MOVBE will be extended to include the
> > “MOVBE reg1, reg2” form (namely, the ModRM.Mod = 3 case) for both
> > opcodes 0xF0 and 0xF1. This extension makes the promotion of BSWAP for
> NDD support unnecessary.
> >
> > I'll add a description of movbe to the patch 2/8 changelog.
> 
> Thanks. I did deduce the connection to BSWAP on the way home yesterday.
> Which made me come up with a (spec) question: Wouldn't it make sense to
> name these
> reg->reg forms BSWAP, not MOVBE? It doesn't really matter that they use
> reg->the
> same encoding as the MOVBE (then mem-only) forms. That would then
> eliminate the concern I raised.
> 

If we extend BSWAP, it needs to add the operation of reading ModRM.Mod. I think this is the benefit of this solution now.

> As to 2/8: I received two v2 thereof, with different titles, and a v2 of 3/8.
> But nothing else. The list archives match this. I may take a brief look, but really
> I'll await a full v2 submission for a proper new review round.
>
After we split a new patch for the EVEX_MAP4_ table, we need to insert a patch into the previous patch. I am afraid that if I change the patch number, the subsequent review will be easily confused, so I have not changed the patch number for the time being.  But for V3 I'll create a new patch  number for it.

Thanks.