On 12/13/23 02:03, Christoph Müllner wrote:
On Wed, Dec 13, 2023 at 9:22 AM Liao Shihua <shi...@iscas.ac.cn> wrote:
In Scalar Crypto Built-In functions, some require immediate parameters,
But register_operand are incorrectly used in the pattern.
E.g.:
__builtin_riscv_aes64ks1i(rs1,1)
Before:
li a5,1
aes64ks1i a0,a0,a5
Assembler messages:
Error: instruction aes64ks1i requires absolute expression
After:
aes64ks1i a0,a0,1
Looks good to me (also tested with rv32 and rv64).
(I was actually surprised that the D03 constraint was not sufficient)
Reviewed-by: Christoph Muellner <christoph.muell...@vrull.eu>
Tested-by: Christoph Muellner <christoph.muell...@vrull.eu>
Nit: I would prefer to separate arguments with a comma followed by a space.
Even if the existing code was not written like that.
E.g. __builtin_riscv_sm4ed(rs1,rs2,1); -> __builtin_riscv_sm4ed(rs1, rs2, 1);
I propose to remove the builtin tests for scalar crypto and scalar bitmanip
as part of the patchset that adds the intrinsic tests (no value in
duplicated tests).
gcc/ChangeLog:
* config/riscv/crypto.md: Use immediate_operand instead of
register_operand.
You should mention the actual patterns changed.
I would strongly recommend adding some tests that out of range cases are
rejected (out of range constants as well as a variable for that last
argument). I did that in my patch from June to fix this problem (which
was never acked/reviewed).
Jeff
