[PATCH] x86: Add {load}/{store} tests for apx instructions.

[Cui, Lili]

> On 03.07.2024 06:01, H.J. Lu wrote:
> > On Wed, Jul 3, 2024, 10:41 AM Cui, Lili <lili.cui@intel.com> wrote:
> >
> >>>> On 02.07.2024 12:29, Cui, Lili wrote:
> >>>>>>>>>> Having reached the end of the file: What about CTESTcc and
> >>> CCMPcc?
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>>> When adding swap test cases for CTESTcc, I found that it does
> >>>>>>> not support
> >>>>>> swapping operands, but test, {evex} test and ctest template insns
> >>>>>> all have D, which does not match the SDM/APX spec, I want to
> >>>>>> remove D for them. I'm a bit unsure if there's any particular
> >>>>>> reason why the legacy test
> >>>> added D ?
> >>>>>>
> >>>>>> Did you look at the commit doing so?
> >>>>>>
> >>>>>
> >>>>> Ok, I found the commit this time, but I'm even more confused
> >>>>> because I
> >>>> didn't find the basis for the second test from SDM.
> >>>>>
> >>>>> -test, 0x84, None, 0, W|CheckRegSize|Modrm|No_sSuf, {
> >>>>> Reg8|Reg16|Reg32|Reg64,
> >>>>>
> >>>>
> >>>
> Reg8|Reg16|Reg32|Reg64|Unspecified|Byte|Word|Dword|Qword|BaseInde
> >>>> x }
> >>>>> -test, 0x84, None, 0, W|CheckRegSize|Modrm|No_sSuf, {
> >>>>> Byte|Word|Dword|Qword|Unspecified|BaseIndex,
> >>>> Reg8|Reg16|Reg32|Reg64 }
> >>>>> +test, 0x84, None, 0, D|W|C|CheckRegSize|Modrm|No_sSuf, {
> >>>>> +Reg8|Reg16|Reg32|Reg64,
> >>>> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex
> >>>>> +}
> >>>>
> >>>> "basis for the second test"? What do you mean? Are you referring to
> >>>> the SDM spelling out only one operand order? That's surely a (very
> >>>> minor) shortcoming there. TEST is commutative, and hence whatever
> >>>> order the programmer likes should be accepted by an assembler.
> >>>
> >>> Yes, I only found "test %ax, (%bx)" and didn't find "test (%bx),
> >>> %ax" in
> >> the
> >>> SDM. And APX spec also doesn’t say " ctest {def=} (%bx), %ax " is
> >> supported.
> >>> Anyway, their assemblers are same.
> >>> I will support "ctestb(%rax),%ecx" and "{evex} test(%rax),%ecx" in
> >> assembler,
> >>> currently "ctestb(%rax),%ecx" does not work, I need to fix it first.
> >>>
> >> $ cat test.s
> >> test (%edx), %eax
> >> test %eax, (%edx)
> >>
> >> $ as --64  test.s -o test.o
> >> $ objdump  -dw -Msuffix test.o
> >> 0000000000000000 <_start>:
> >>    0:   67 85 02                testl  %eax,(%edx)
> >>    3:   67 85 02                testl  %eax,(%edx)
> >>
> >> Hi HJ,
> >>
> >> Do you remember why we supported "test (%edx), %eax" in the original
> >> version? I'm curious about it.
> >
> > I don't remember why.  I think we should follow SDM.
> 
> We should follow what makes sense in written code. Other assemblers permit
> either order of the operands, when their order doesn't matter.
> So should gas do. The fact that the SDM spells out only one order of operands
> is (presumably) connected to there being only one encoding (unlike for e.g.
> CMP). This does not mean we should be unfriendly to people and force a
> purely stylistic restriction onto them. And just to mention it: For TEST both
> forms were supported virtually forever.
> Quite likely simply because, as said, other assemblers to do, too.
> 

Yes, I found that LLVM and NSAM also support "test (%edx), %eax", and llvm has just created a jira to follow our decision (They also want to remove it instead of continuing to make similar extensions to ctest, but finally it depends on us).

Personally, since no one knows the original intention of this alias, it seems more like a bug to me, because I can't see its benefit, SDM clearly says that "When two operands are present in an arithmetic or logical instruction, the right operand is the source and the left operand is the destination". "test eax, [edx]" is very confusing to me. Someone writing like this would mistakenly think that dest is a register, but in fact it is a memory. I think this is a trap, we should report an error for it.

If you insist on not deleting "test (%edx), %eax",  and extending it to ctest, I will also agree. I don't think anyone would use it that way.

Thanks,
Lili.