[v2,0/2] RISC-V: add gcc support for Scalar Cryptography v1.0.0-rc6

  From: SiYu Wu <siyu@isrc.iscas.ac.cn>

This patch add gcc backend support for RISC-V Scalar Cryptography 
Extension (k-ext), including machine description, builtins defines and 
testcases for each k-ext's subset.

A note about Zbkx: The Zbkx should be implemented in bitmanip's Zbp, but 
since zbp is not included in the bitmanip spec v1.0, and crypto's v1.0 
release will earlier than bitmanip's next release, so for now we 
implementing it here.

Version logs:

v2: As Kito mentions, now this patch only includes the arch string related 
stuff, the builtins and md changes is not included, waiting for the builtin
and intrinsic added to the spec. Also removed the unnecessary patches and add
Changelogs.


SiYu Wu (2):
  RISC-V: Add option defines for Scalar Cryptography
  RISC-V: Add implied defines of Zk, Zkn and Zks

 gcc/common/config/riscv/riscv-common.c | 38 +++++++++++++++++++++++++-
 gcc/config/riscv/arch-canonicalize     | 16 ++++++++++-
 gcc/config/riscv/riscv-opts.h          | 22 +++++++++++++++
 gcc/config/riscv/riscv.opt             |  3 ++
 4 files changed, 77 insertions(+), 2 deletions(-)
  

[Changing to Jim's new address]

On Mon, 22 Nov 2021 00:19:08 PST (-0800), siyu@isrc.iscas.ac.cn wrote:
> From: SiYu Wu <siyu@isrc.iscas.ac.cn>
>
> This patch add gcc backend support for RISC-V Scalar Cryptography
> Extension (k-ext), including machine description, builtins defines and
> testcases for each k-ext's subset.
>
> A note about Zbkx: The Zbkx should be implemented in bitmanip's Zbp, but
> since zbp is not included in the bitmanip spec v1.0, and crypto's v1.0
> release will earlier than bitmanip's next release, so for now we
> implementing it here.
>
> Version logs:
>
> v2: As Kito mentions, now this patch only includes the arch string related
> stuff, the builtins and md changes is not included, waiting for the builtin
> and intrinsic added to the spec. Also removed the unnecessary patches and add
> Changelogs.

I don't think there's anything wrong with what's here, but IMO we should 
hold off on merging until GCC does something with these extensions.  

IIUC all this enables is passing "-march=*Zk*" instead of 
"-Wa,-march=*Zk*", and while that is useful I'm worried it'll just make 
more of a headache for users who lose a simple way to detect the 
intrinsics.  IMO forcing users to pass -Wa properly encodes the "GCC 
doesn't support these, but binutils does" scenario pretty sanely, and 
users doing things at this level of complexity should be used to that 
already because it happens somewhat frequently.

I'm not sure if I'm missing some use case this for this, though.

> SiYu Wu (2):
>   RISC-V: Add option defines for Scalar Cryptography
>   RISC-V: Add implied defines of Zk, Zkn and Zks
>
>  gcc/common/config/riscv/riscv-common.c | 38 +++++++++++++++++++++++++-
>  gcc/config/riscv/arch-canonicalize     | 16 ++++++++++-
>  gcc/config/riscv/riscv-opts.h          | 22 +++++++++++++++
>  gcc/config/riscv/riscv.opt             |  3 ++
>  4 files changed, 77 insertions(+), 2 deletions(-)
  

I would prefer to accept those patchset even with no builtin function
or intrinsic function yet,
this not only add the support of -march option, but also introduce the
predefined macros like __riscv_zk*,
which could be used in *.S file to check if those instructions are
available or not.


On Wed, Nov 24, 2021 at 11:23 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> [Changing to Jim's new address]
>
> On Mon, 22 Nov 2021 00:19:08 PST (-0800), siyu@isrc.iscas.ac.cn wrote:
> > From: SiYu Wu <siyu@isrc.iscas.ac.cn>
> >
> > This patch add gcc backend support for RISC-V Scalar Cryptography
> > Extension (k-ext), including machine description, builtins defines and
> > testcases for each k-ext's subset.
> >
> > A note about Zbkx: The Zbkx should be implemented in bitmanip's Zbp, but
> > since zbp is not included in the bitmanip spec v1.0, and crypto's v1.0
> > release will earlier than bitmanip's next release, so for now we
> > implementing it here.
> >
> > Version logs:
> >
> > v2: As Kito mentions, now this patch only includes the arch string related
> > stuff, the builtins and md changes is not included, waiting for the builtin
> > and intrinsic added to the spec. Also removed the unnecessary patches and add
> > Changelogs.
>
> I don't think there's anything wrong with what's here, but IMO we should
> hold off on merging until GCC does something with these extensions.
>
> IIUC all this enables is passing "-march=*Zk*" instead of
> "-Wa,-march=*Zk*", and while that is useful I'm worried it'll just make
> more of a headache for users who lose a simple way to detect the
> intrinsics.  IMO forcing users to pass -Wa properly encodes the "GCC
> doesn't support these, but binutils does" scenario pretty sanely, and
> users doing things at this level of complexity should be used to that
> already because it happens somewhat frequently.
>
> I'm not sure if I'm missing some use case this for this, though.
>
> > SiYu Wu (2):
> >   RISC-V: Add option defines for Scalar Cryptography
> >   RISC-V: Add implied defines of Zk, Zkn and Zks
> >
> >  gcc/common/config/riscv/riscv-common.c | 38 +++++++++++++++++++++++++-
> >  gcc/config/riscv/arch-canonicalize     | 16 ++++++++++-
> >  gcc/config/riscv/riscv-opts.h          | 22 +++++++++++++++
> >  gcc/config/riscv/riscv.opt             |  3 ++
> >  4 files changed, 77 insertions(+), 2 deletions(-)
  

On Wed, 24 Nov 2021 02:00:33 PST (-0800), Kito Cheng wrote:
> I would prefer to accept those patchset even with no builtin function
> or intrinsic function yet,
> this not only add the support of -march option, but also introduce the
> predefined macros like __riscv_zk*,
> which could be used in *.S file to check if those instructions are
> available or not.

That makes sense, I guess I hadn't thought of that use case.

> On Wed, Nov 24, 2021 at 11:23 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>>
>> [Changing to Jim's new address]
>>
>> On Mon, 22 Nov 2021 00:19:08 PST (-0800), siyu@isrc.iscas.ac.cn wrote:
>> > From: SiYu Wu <siyu@isrc.iscas.ac.cn>
>> >
>> > This patch add gcc backend support for RISC-V Scalar Cryptography
>> > Extension (k-ext), including machine description, builtins defines and
>> > testcases for each k-ext's subset.
>> >
>> > A note about Zbkx: The Zbkx should be implemented in bitmanip's Zbp, but
>> > since zbp is not included in the bitmanip spec v1.0, and crypto's v1.0
>> > release will earlier than bitmanip's next release, so for now we
>> > implementing it here.
>> >
>> > Version logs:
>> >
>> > v2: As Kito mentions, now this patch only includes the arch string related
>> > stuff, the builtins and md changes is not included, waiting for the builtin
>> > and intrinsic added to the spec. Also removed the unnecessary patches and add
>> > Changelogs.
>>
>> I don't think there's anything wrong with what's here, but IMO we should
>> hold off on merging until GCC does something with these extensions.
>>
>> IIUC all this enables is passing "-march=*Zk*" instead of
>> "-Wa,-march=*Zk*", and while that is useful I'm worried it'll just make
>> more of a headache for users who lose a simple way to detect the
>> intrinsics.  IMO forcing users to pass -Wa properly encodes the "GCC
>> doesn't support these, but binutils does" scenario pretty sanely, and
>> users doing things at this level of complexity should be used to that
>> already because it happens somewhat frequently.
>>
>> I'm not sure if I'm missing some use case this for this, though.
>>
>> > SiYu Wu (2):
>> >   RISC-V: Add option defines for Scalar Cryptography
>> >   RISC-V: Add implied defines of Zk, Zkn and Zks
>> >
>> >  gcc/common/config/riscv/riscv-common.c | 38 +++++++++++++++++++++++++-
>> >  gcc/config/riscv/arch-canonicalize     | 16 ++++++++++-
>> >  gcc/config/riscv/riscv-opts.h          | 22 +++++++++++++++
>> >  gcc/config/riscv/riscv.opt             |  3 ++
>> >  4 files changed, 77 insertions(+), 2 deletions(-)
  

Hi SiYu:

Committed, thanks!

On Thu, Nov 25, 2021 at 12:42 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Wed, 24 Nov 2021 02:00:33 PST (-0800), Kito Cheng wrote:
> > I would prefer to accept those patchset even with no builtin function
> > or intrinsic function yet,
> > this not only add the support of -march option, but also introduce the
> > predefined macros like __riscv_zk*,
> > which could be used in *.S file to check if those instructions are
> > available or not.
>
> That makes sense, I guess I hadn't thought of that use case.
>
> > On Wed, Nov 24, 2021 at 11:23 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
> >>
> >> [Changing to Jim's new address]
> >>
> >> On Mon, 22 Nov 2021 00:19:08 PST (-0800), siyu@isrc.iscas.ac.cn wrote:
> >> > From: SiYu Wu <siyu@isrc.iscas.ac.cn>
> >> >
> >> > This patch add gcc backend support for RISC-V Scalar Cryptography
> >> > Extension (k-ext), including machine description, builtins defines and
> >> > testcases for each k-ext's subset.
> >> >
> >> > A note about Zbkx: The Zbkx should be implemented in bitmanip's Zbp, but
> >> > since zbp is not included in the bitmanip spec v1.0, and crypto's v1.0
> >> > release will earlier than bitmanip's next release, so for now we
> >> > implementing it here.
> >> >
> >> > Version logs:
> >> >
> >> > v2: As Kito mentions, now this patch only includes the arch string related
> >> > stuff, the builtins and md changes is not included, waiting for the builtin
> >> > and intrinsic added to the spec. Also removed the unnecessary patches and add
> >> > Changelogs.
> >>
> >> I don't think there's anything wrong with what's here, but IMO we should
> >> hold off on merging until GCC does something with these extensions.
> >>
> >> IIUC all this enables is passing "-march=*Zk*" instead of
> >> "-Wa,-march=*Zk*", and while that is useful I'm worried it'll just make
> >> more of a headache for users who lose a simple way to detect the
> >> intrinsics.  IMO forcing users to pass -Wa properly encodes the "GCC
> >> doesn't support these, but binutils does" scenario pretty sanely, and
> >> users doing things at this level of complexity should be used to that
> >> already because it happens somewhat frequently.
> >>
> >> I'm not sure if I'm missing some use case this for this, though.
> >>
> >> > SiYu Wu (2):
> >> >   RISC-V: Add option defines for Scalar Cryptography
> >> >   RISC-V: Add implied defines of Zk, Zkn and Zks
> >> >
> >> >  gcc/common/config/riscv/riscv-common.c | 38 +++++++++++++++++++++++++-
> >> >  gcc/config/riscv/arch-canonicalize     | 16 ++++++++++-
> >> >  gcc/config/riscv/riscv-opts.h          | 22 +++++++++++++++
> >> >  gcc/config/riscv/riscv.opt             |  3 ++
> >> >  4 files changed, 77 insertions(+), 2 deletions(-)