On 1/20/25 2:18 AM, pan2...@intel.com wrote:
From: Pan Li <pan2...@intel.com>
This patch would like to fix the wroing code generation for the scalar
signed SAT_SUB. The input can be QI/HI/SI/DI while the alu like sub
can only work on Xmode, thus we need to make sure the value of input
are well signed-extended at first. But the gen_lowpart will generate
something like lbu which will perform the zero extended.
The below test suites are passed for this patch.
* The rv64gcv fully regression test.
Note we also notice some refinement like to support const_int for input
or similar issue for ssadd and/or sstruct. But we would like to fix
it by another patch(es).
PR target/117688
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_gen_sign_extend_rtx): Add new func
to make sure the op is signed extended to Xmode.
(riscv_expand_sssub): Leverage above func to perform sign extend
if not Xmode.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/pr117688-run-1-s16.c: New test.
* gcc.target/riscv/pr117688-run-1-s32.c: New test.
* gcc.target/riscv/pr117688-run-1-s8.c: New test.
* gcc.target/riscv/pr117688.h: New test.
So a bit of high level background why this is needed would be helpful.
My suspicion is that we're using a functional interface, but without
going through any of the usual parameter/argument setup routines. So a
sub-word object hasn't gone through a mandatory sign extension.
+/* Generate a REG rtx of Xmode from the given rtx and mode.
+ The rtx x can be REG (QI/HI/SI/DI).
+ The machine_mode mode is the original mode from define pattern.
+
+ If rtx is REG and Xmode, the RTX x will be returned directly.
+
+ If rtx is REG and non-Xmode, the sign extended to new REG of Xmode will be
+ returned.
+
+ Then the underlying expanding can perform the code generation based on
+ the REG rtx of Xmode, instead of taking care of these in expand func. */
+
+static rtx
+riscv_gen_sign_extend_rtx (rtx x, machine_mode mode)
+{
+ if (mode == Xmode)
+ return x;
+
+ rtx xmode_reg = gen_reg_rtx (Xmode);
+ riscv_emit_unary (SIGN_EXTEND, xmode_reg, x);
+
+ return xmode_reg;
+}
You might be able to (ab)use riscv_expand_comparands rather than making
a new function. You'd probably want to rename it in that case.
The other high level question, wouldn't this be needed for other signed
cases, not just those going through expand_sssub?
jeff
