Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug?

[Nelson Chu <nelson@rivosinc.com>]

[-- Attachment #1: Type: text/plain, Size: 6019 bytes --]

Is the following expected?  Seems "vd is any" also has the same issue.

Nelson

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c

index 959cbbc32a5..e49b34fd524 100644

--- a/gas/config/tc-riscv.c

+++ b/gas/config/tc-riscv.c

@@ -1966,13 +1966,13 @@ vector_macro (struct riscv_cl_insn *ip)

          /* Masked.  Have vtemp to avoid overlap constraints.  */

          if (vd == vm)

            {

-             macro_build (NULL, "vmslt.vx", "Vd,Vt,s", vtemp, vs2, vs1);

+             macro_build (NULL, "vmslt.vx", "Vd,Vt,sVm", vtemp, vs2, vs1,
-1);

              macro_build (NULL, "vmandnot.mm", "Vd,Vt,Vs", vd, vm, vtemp);

            }

          else

            {

              /* Preserve the value of vd if not updating by vm.  */

-             macro_build (NULL, "vmslt.vx", "Vd,Vt,s", vtemp, vs2, vs1);

+             macro_build (NULL, "vmslt.vx", "Vd,Vt,sVm", vtemp, vs2, vs1,
-1);

              macro_build (NULL, "vmandnot.mm", "Vd,Vt,Vs", vtemp, vm,
vtemp);

              macro_build (NULL, "vmandnot.mm", "Vd,Vt,Vs", vd, vd, vm);

              macro_build (NULL, "vmor.mm", "Vd,Vt,Vs", vd, vtemp, vd);

@@ -2001,13 +2001,13 @@ vector_macro (struct riscv_cl_insn *ip)

          /* Masked.  Have vtemp to avoid overlap constraints.  */

          if (vd == vm)

            {

-             macro_build (NULL, "vmsltu.vx", "Vd,Vt,s", vtemp, vs2, vs1);

+             macro_build (NULL, "vmsltu.vx", "Vd,Vt,sVm", vtemp, vs2, vs1,
-1);

              macro_build (NULL, "vmandnot.mm", "Vd,Vt,Vs", vd, vm, vtemp);

            }

          else

            {

              /* Preserve the value of vd if not updating by vm.  */

-             macro_build (NULL, "vmsltu.vx", "Vd,Vt,s", vtemp, vs2, vs1);

+             macro_build (NULL, "vmsltu.vx", "Vd,Vt,sVm", vtemp, vs2, vs1,
-1);

              macro_build (NULL, "vmandnot.mm", "Vd,Vt,Vs", vtemp, vm,
vtemp);

              macro_build (NULL, "vmandnot.mm", "Vd,Vt,Vs", vd, vd, vm);

              macro_build (NULL, "vmor.mm", "Vd,Vt,Vs", vd, vtemp, vd);

diff --git a/gas/testsuite/gas/riscv/vector-insns-vmsgtvx.d
b/gas/testsuite/gas/riscv/vector-insns-vmsgtvx.d

index dcc951a3cbf..aa633e357c9 100644

--- a/gas/testsuite/gas/riscv/vector-insns-vmsgtvx.d

+++ b/gas/testsuite/gas/riscv/vector-insns-vmsgtvx.d

@@ -11,9 +11,9 @@ Disassembly of section .text:

 [      ]+[0-9a-f]+:[   ]+76422257[     ]+vmnot.m[      ]+v4,v4

 [      ]+[0-9a-f]+:[   ]+6cc64457[     ]+vmslt.vx[     ]+v8,v12,a2,v0.t

 [      ]+[0-9a-f]+:[   ]+6e802457[     ]+vmxor.mm[     ]+v8,v8,v0

-[      ]+[0-9a-f]+:[   ]+6c85c657[     ]+vmslt.vx[     ]+v12,v8,a1,v0.t

+[      ]+[0-9a-f]+:[   ]+6e85c657[     ]+vmslt.vx[     ]+v12,v8,a1

 [      ]+[0-9a-f]+:[   ]+62062057[     ]+vmandn.mm[    ]+v0,v0,v12

-[      ]+[0-9a-f]+:[   ]+6c85c657[     ]+vmslt.vx[     ]+v12,v8,a1,v0.t

+[      ]+[0-9a-f]+:[   ]+6e85c657[     ]+vmslt.vx[     ]+v12,v8,a1

 [      ]+[0-9a-f]+:[   ]+62062657[     ]+vmandn.mm[    ]+v12,v0,v12

 [      ]+[0-9a-f]+:[   ]+62402257[     ]+vmandn.mm[    ]+v4,v4,v0

 [      ]+[0-9a-f]+:[   ]+6ac22257[     ]+vmor.mm[      ]+v4,v12,v4

@@ -21,9 +21,9 @@ Disassembly of section .text:

 [      ]+[0-9a-f]+:[   ]+76422257[     ]+vmnot.m[      ]+v4,v4

 [      ]+[0-9a-f]+:[   ]+68c64457[     ]+vmsltu.vx[    ]+v8,v12,a2,v0.t

 [      ]+[0-9a-f]+:[   ]+6e802457[     ]+vmxor.mm[     ]+v8,v8,v0

-[      ]+[0-9a-f]+:[   ]+6885c657[     ]+vmsltu.vx[    ]+v12,v8,a1,v0.t

+[      ]+[0-9a-f]+:[   ]+6a85c657[     ]+vmsltu.vx[    ]+v12,v8,a1

 [      ]+[0-9a-f]+:[   ]+62062057[     ]+vmandn.mm[    ]+v0,v0,v12

-[      ]+[0-9a-f]+:[   ]+6885c657[     ]+vmsltu.vx[    ]+v12,v8,a1,v0.t

+[      ]+[0-9a-f]+:[   ]+6a85c657[     ]+vmsltu.vx[    ]+v12,v8,a1

 [      ]+[0-9a-f]+:[   ]+62062657[     ]+vmandn.mm[    ]+v12,v0,v12

 [      ]+[0-9a-f]+:[   ]+62402257[     ]+vmandn.mm[    ]+v4,v4,v0

 [      ]+[0-9a-f]+:[   ]+6ac22257[     ]+vmor.mm[      ]+v4,v12,v4

On Tue, Aug 29, 2023 at 3:43 PM Jan Beulich <jbeulich@suse.com> wrote:

> On 29.08.2023 08:52, im Kiva via Binutils wrote:
> > Hi maintainers,
> >
> > I discovered that GNU Assembler (as) lowers `vmsge.vx` and `vmsgeu.vx`
> (pseudo
> > instructions from RISC-V Vector Extension [1]) when the destination
> > register is v0 as follows:
> >
> > vmsge{u}.vx v0, v4, a0, v0.t, v2
> >
> > will be expanded to:
> >
> > vmslt{u}.vx v2, v4, a0, v0.t
> > vmandn.mm v0, v0, v2
> >
> > You can inspect the lowering result with Godbolt [2].
> > However, according to the Vector specification [1] page 52. The
> "desugared"
> > `vmslt{u}.vx` is not masked:
> >
> >> masked va >= x, vd == v0
> >>   pseudoinstruction: vmsge{u}.vx vd, va, x, v0.t, vt
> >>   expansion: vmslt{u}.vx vt, va, x; vmandn.mm vd, vd, vt
> >
> > So the spec-expected result of the example above should be:
> > vmslt{u}.vx v2, v4, a0 <-- no v0.t here
> > vmandn.mm v0, v0, v2
> >
> > I thus submitted a patch to the LLVM [3], and it was accepted recently.
> >
> > I am wondering if binutils considers it a bug, or if it is just
> intentional
> > because of some historical and compatibility reasons.
>
> I think this wants fixing alike in binutils: From looking at
> vector_macro(),
> it appears that emitting the masked form is merely an accident resulting
> from the inverted encoding of "masking". In particular, if masking was
> indeed meant, I expect code there would be
>
>                   if (vd == vm)
>                     {
>                       macro_build (NULL, "vmslt.vx", "Vd,Vt,sVm", vtemp,
> vs2, vs1, vm);
>                       macro_build (NULL, "vmandnot.mm", "Vd,Vt,Vs", vd,
> vm, vtemp);
>                     }
>                   else
>                     ...
>
> much like it is a few lines down from there. (Apparently the "else" path
> omitted above is similarly affected.)
>
> Jan
>