[Bug target/110201] New: RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[Bug target/110201] New: RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[craig.topper at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

            Bug ID: 110201
           Summary: RISC-V: __builtin_riscv_sm4ks and
                    __builtin_riscv_sm4ed produce invalid assembly
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: craig.topper at gmail dot com
  Target Milestone: ---

The __builtin_riscv_sm4ks and __builtin_riscv_sm4ed builtins don't enforce that
the byte select should be an immediate. if an immediate provided they still use
a register.

int32_t foo1(int32_t rs1, int32_t rs2, int bs)
{
    return __builtin_riscv_sm4ks(rs1,rs2,bs);
}

int32_t foo2(int32_t rs1, int32_t rs2, int bs)
{
    return __builtin_riscv_sm4ed(rs1,rs2,bs);
}

int32_t foo3(int32_t rs1, int32_t rs2, int bs)
{
    return __builtin_riscv_sm4ks(rs1,rs2,0);
}

int32_t foo4(int32_t rs1, int32_t rs2, int bs)
{
    return __builtin_riscv_sm4ed(rs1,rs2,0);
}

https://godbolt.org/z/jadKva9M9

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-06-19
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Jeffrey A. Law <law at gcc dot gnu.org> ---
It looks like some of the aes patterns have the same problem.  It may just have
been Liao not understanding the difference between an operand constraint and an
operand predicate.

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[palmer at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

--- Comment #2 from palmer at gcc dot gnu.org ---
Do you guys have a test suite for these, or did you just happen to run into it?
 The intrinsic testing has been a bit of a blind spot in GCC land.

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[craig.topper at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

--- Comment #3 from Craig Topper <craig.topper at gmail dot com> ---
I don't have a testsuite. I saw that gcc had crypto builtins and I happened to
noticed the tests in gcc weren't passing constant arguments.

We also have a divergence in names between clang and gcc for some crypto
builtins. We really need to define a scalar crypto intrinsic header file.

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

--- Comment #4 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Yea, the tests aren't great.  They'll be better shortly.  They'll test
non-constant arguments and out-of-range constants, expecting a suitable
diagnostic.  They'll also test the extrema of valid constants.

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[palmer at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

--- Comment #5 from palmer at gcc dot gnu.org ---
(In reply to Jeffrey A. Law from comment #4)
> Yea, the tests aren't great.  They'll be better shortly.  They'll test
> non-constant arguments and out-of-range constants, expecting a suitable
> diagnostic.  They'll also test the extrema of valid constants.

Awesome, thanks!

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[palmer at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

--- Comment #6 from palmer at gcc dot gnu.org ---
(In reply to Craig Topper from comment #3)
> I don't have a testsuite. I saw that gcc had crypto builtins and I happened
> to noticed the tests in gcc weren't passing constant arguments.
> 
> We also have a divergence in names between clang and gcc for some crypto
> builtins. We really need to define a scalar crypto intrinsic header file.

OK, let's try and get that sorted out?  We're generally not supposed to be
merging intrinsics without some sort of spec to point at, but we did a pretty
poor job at that for the V intrinsics and it looks like we've slipped a bit
here too.

Unless I'm missing something we haven't released GCC with the crypto intrinsics
yet, so we should be safe to fix bugs there as they come up.

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[craig.topper at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

--- Comment #7 from Craig Topper <craig.topper at gmail dot com> ---
Here is my attempt and defining scalar crypto intrinsics
https://github.com/riscv-non-isa/riscv-c-api-doc/pull/44

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jeff Law <law@gcc.gnu.org>:

https://gcc.gnu.org/g:082835836cf763e74ed2fdf9645ac4f1f94d6d4f

commit r14-6607-g082835836cf763e74ed2fdf9645ac4f1f94d6d4f
Author: Jeff Law <jlaw@ventanamicro.com>
Date:   Fri Dec 15 14:19:25 2023 -0700

    Re: [PATCH] RISC-V: fix scalar crypto patterns

    A handful of the scalar crypto instructions are supposed to take a
    constant integer argument 0..3 inclusive and one should accept 0..10.
    A suitable constraint was created and used for this purpose (D03 and DsA),
    but the operand's predicate is "register_operand".  That's just wrong.

    This patch adds a new predicates "const_0_3_operand" and
"const_0_10_operand"
    and fixes the relevant insns to use the appropriate predicate.   It drops
the
    now unnecessary constraints.

    The testsuite was broken in a way that made it consistent with the
    compiler, so the tests passed, when they really should have been issuing
    errors all along.

    This patch adjusts the existing tests so that they all expect a
    diagnostic on the invalid operand usage (including out of range
    constants).  It adds new tests with proper constants, testing the
    extremes of valid values.

            PR target/110201

    gcc/

            * config/riscv/constraints.md (D03, DsA): Remove unused
constraints.
            * config/riscv/predicates.md (const_0_3_operand): New predicate.
            (const_0_10_operand): Likewise.
            * config/riscv/crypto.md (riscv_aes32dsi): Use new predicate.  Drop
            unnecessary constraint.
            (riscv_aes32dsmi, riscv_aes64im, riscv_aes32esi): Likewise.
            (riscv_aes32esmi, *riscv_<sm4_op>_si): Likewise.
            (riscv_<sm4_op>_di_extend, riscv_<sm4_op>_si): Likewise.

    gcc/testsuite
            * gcc.target/riscv/zknd32.c: Verify diagnostics are issued for
            invalid builtin arguments.
            * gcc.target/riscv/zknd64.c: Likewise.
            * gcc.target/riscv/zkne32.c: Likewise.
            * gcc.target/riscv/zkne64.c: Likewise.
            * gcc.target/riscv/zksed32.c: Likewise.
            * gcc.target/riscv/zksed64.c: Likewise.
            * gcc.target/riscv/zknd32-2.c: New test
            * gcc.target/riscv/zknd64-2.c: Likewise.
            * gcc.target/riscv/zkne32-2.c: Likewise.
            * gcc.target/riscv/zkne64-2.c: Likewise.
            * gcc.target/riscv/zksed32-2.c: Likewise.
            * gcc.target/riscv/zksed64-2.c: Likewise.

            Co-authored-by: Liao Shihua <shihua@iscas.ac.cn>

[Bug target/110201] RISC-V: __builtin_riscv_sm4ks and __builtin_riscv_sm4ed produce invalid assembly

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110201

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #9 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Should (finally) be fixed on the trunk.