* [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug?
@ 2023-08-29 6:52 im Kiva
2023-08-29 7:43 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: im Kiva @ 2023-08-29 6:52 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
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.
Thanks.
Best regards
Kiva Oyama
[1]: https://github.com/riscv/riscv-v-spec/releases/tag/v1.0
[2]: https://godbolt.org/z/aszc5d8sh
[3]: https://reviews.llvm.org/D158392
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? 2023-08-29 6:52 [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? im Kiva @ 2023-08-29 7:43 ` Jan Beulich 2023-08-29 7:59 ` Nelson Chu 0 siblings, 1 reply; 8+ messages in thread From: Jan Beulich @ 2023-08-29 7:43 UTC (permalink / raw) To: im Kiva, Nelson Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? 2023-08-29 7:43 ` Jan Beulich @ 2023-08-29 7:59 ` Nelson Chu 2023-08-29 8:52 ` Jan Beulich 0 siblings, 1 reply; 8+ messages in thread From: Nelson Chu @ 2023-08-29 7:59 UTC (permalink / raw) To: Jan Beulich; +Cc: im Kiva, binutils [-- 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? 2023-08-29 7:59 ` Nelson Chu @ 2023-08-29 8:52 ` Jan Beulich 2023-08-30 2:51 ` Nelson Chu 0 siblings, 1 reply; 8+ messages in thread From: Jan Beulich @ 2023-08-29 8:52 UTC (permalink / raw) To: Nelson Chu, im Kiva; +Cc: binutils On 29.08.2023 09:59, Nelson Chu wrote: > Is the following expected? I think so, but Kiva - please confirm. > Seems "vd is any" also has the same issue. Right, that's what ... >> 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.) ... I was referring to with the parenthesized sentence. Jan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? 2023-08-29 8:52 ` Jan Beulich @ 2023-08-30 2:51 ` Nelson Chu 2023-08-30 5:50 ` Jan Beulich 0 siblings, 1 reply; 8+ messages in thread From: Nelson Chu @ 2023-08-30 2:51 UTC (permalink / raw) To: Jan Beulich; +Cc: im Kiva, binutils [-- Attachment #1: Type: text/plain, Size: 1312 bytes --] Hi Jan, Thanks for pointing that out. Hi Kiva, Maybe you can send a better patch for this issue if you are interested, including what Jan suggested. Thanks :-) Nelson On Tue, Aug 29, 2023 at 4:52 PM Jan Beulich <jbeulich@suse.com> wrote: > On 29.08.2023 09:59, Nelson Chu wrote: > > Is the following expected? > > I think so, but Kiva - please confirm. > > > Seems "vd is any" also has the same issue. > > Right, that's what ... > > >> 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.) > > ... I was referring to with the parenthesized sentence. > > Jan > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? 2023-08-30 2:51 ` Nelson Chu @ 2023-08-30 5:50 ` Jan Beulich 2023-09-01 6:37 ` Kiva Oyama 0 siblings, 1 reply; 8+ messages in thread From: Jan Beulich @ 2023-08-30 5:50 UTC (permalink / raw) To: Nelson Chu; +Cc: im Kiva, binutils On 30.08.2023 04:51, Nelson Chu wrote: > Hi Jan, > Thanks for pointing that out. > > Hi Kiva, > Maybe you can send a better patch for this issue if you are interested, > including what Jan suggested. Thanks :-) Hmm, both of your replies above make me suspect a misunderstanding: I think your patch does exactly what is needed. Jan > On Tue, Aug 29, 2023 at 4:52 PM Jan Beulich <jbeulich@suse.com> wrote: > >> On 29.08.2023 09:59, Nelson Chu wrote: >>> Is the following expected? >> >> I think so, but Kiva - please confirm. >> >>> Seems "vd is any" also has the same issue. >> >> Right, that's what ... >> >>>> 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.) >> >> ... I was referring to with the parenthesized sentence. >> >> Jan >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? 2023-08-30 5:50 ` Jan Beulich @ 2023-09-01 6:37 ` Kiva Oyama 2023-09-01 8:03 ` Nelson Chu 0 siblings, 1 reply; 8+ messages in thread From: Kiva Oyama @ 2023-09-01 6:37 UTC (permalink / raw) To: Jan Beulich; +Cc: Nelson Chu, binutils [-- Attachment #1: Type: text/plain, Size: 1988 bytes --] > Is the following expected? Yes. > Seems "vd is any" also has the same issue. I rechecked the spec, for every masked vmsge{u}.vx, the lowered instruction is only masked when `vd != v0`. So your patch does fix these two problems. Thank you for clarifying. Best Regards, Kiva Oyama On Wed, Aug 30, 2023 at 1:51 PM Jan Beulich <jbeulich@suse.com> wrote: > On 30.08.2023 04:51, Nelson Chu wrote: > > Hi Jan, > > Thanks for pointing that out. > > > > Hi Kiva, > > Maybe you can send a better patch for this issue if you are interested, > > including what Jan suggested. Thanks :-) > > Hmm, both of your replies above make me suspect a misunderstanding: I > think your patch does exactly what is needed. > > Jan > > > On Tue, Aug 29, 2023 at 4:52 PM Jan Beulich <jbeulich@suse.com> wrote: > > > >> On 29.08.2023 09:59, Nelson Chu wrote: > >>> Is the following expected? > >> > >> I think so, but Kiva - please confirm. > >> > >>> Seems "vd is any" also has the same issue. > >> > >> Right, that's what ... > >> > >>>> 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.) > >> > >> ... I was referring to with the parenthesized sentence. > >> > >> Jan > >> > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? 2023-09-01 6:37 ` Kiva Oyama @ 2023-09-01 8:03 ` Nelson Chu 0 siblings, 0 replies; 8+ messages in thread From: Nelson Chu @ 2023-09-01 8:03 UTC (permalink / raw) To: Kiva Oyama; +Cc: Jan Beulich, binutils [-- Attachment #1: Type: text/plain, Size: 2223 bytes --] Thanks for both your clarifications, committed. Thanks Nelson On Fri, Sep 1, 2023 at 2:37 PM Kiva Oyama <libkernelpanic@gmail.com> wrote: > > Is the following expected? > > Yes. > > > Seems "vd is any" also has the same issue. > > I rechecked the spec, for every masked vmsge{u}.vx, the lowered > instruction is only masked when `vd != v0`. > So your patch does fix these two problems. > > Thank you for clarifying. > > Best Regards, > Kiva Oyama > > On Wed, Aug 30, 2023 at 1:51 PM Jan Beulich <jbeulich@suse.com> wrote: > >> On 30.08.2023 04:51, Nelson Chu wrote: >> > Hi Jan, >> > Thanks for pointing that out. >> > >> > Hi Kiva, >> > Maybe you can send a better patch for this issue if you are interested, >> > including what Jan suggested. Thanks :-) >> >> Hmm, both of your replies above make me suspect a misunderstanding: I >> think your patch does exactly what is needed. >> >> Jan >> >> > On Tue, Aug 29, 2023 at 4:52 PM Jan Beulich <jbeulich@suse.com> wrote: >> > >> >> On 29.08.2023 09:59, Nelson Chu wrote: >> >>> Is the following expected? >> >> >> >> I think so, but Kiva - please confirm. >> >> >> >>> Seems "vd is any" also has the same issue. >> >> >> >> Right, that's what ... >> >> >> >>>> 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.) >> >> >> >> ... I was referring to with the parenthesized sentence. >> >> >> >> Jan >> >> >> > >> >> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-01 8:03 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-29 6:52 [RISCV] [GNU AS] Possible `vmsge{u}.vx` instruction lowering bug? im Kiva 2023-08-29 7:43 ` Jan Beulich 2023-08-29 7:59 ` Nelson Chu 2023-08-29 8:52 ` Jan Beulich 2023-08-30 2:51 ` Nelson Chu 2023-08-30 5:50 ` Jan Beulich 2023-09-01 6:37 ` Kiva Oyama 2023-09-01 8:03 ` Nelson Chu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).