Thanks Jeff.
> So for the tests, why are we forcing matching of the assembly code for
> the entire function? That must makes for a fragile test as we may
> change various aspects of code generation over time.
> If the point of the patch is to detect SAT_ADD in more cases, then the
> better and more stable test is to verify the existence of SAT_ADD the
> appropriate number of times in the .optimized dump.
> IMHO we really don't want this kind of whole function assembly matching.
> Pan, do you have any further comments here? Do you have strong opinions
> on whether or not we want to be doing this kind of assembly output
> testing or not?
Unlike vector we have vsadd for asm check, the scalar SAT_* will expand to
sorts of branchless codes.
So I add the function body check with no-schedule-insns. However, we have run
test for the same
scenarios which may also indicates the code-gen for SAT_ADD is correct.
Given that it is totally OK to drop that whole function check. How about keep
this series as is and I
can help to drop all the similar check of scalar in another thread?
Pan
-----Original Message-----
From: Jeff Law <jeffreya...@gmail.com>
Sent: Wednesday, May 21, 2025 2:05 AM
To: Li Xu <xu...@eswincomputing.com>; gcc-patches@gcc.gnu.org
Cc: kito.ch...@gmail.com; richard.guent...@gmail.com; tamar.christ...@arm.com;
juzhe.zh...@rivai.ai; Li, Pan2 <pan2...@intel.com>
Subject: Re: [PATCH 2/2] RISC-V:Add testcases for signed .SAT_ADD IMM form 1
with IMM = -1.
On 5/19/25 2:41 AM, Li Xu wrote:
> From: xuli <xu...@eswincomputing.com>
>
> This patch adds testcase for form1, as shown below:
>
> T __attribute__((noinline)) \
> sat_s_add_imm_##T##_fmt_1##_##INDEX (T x) \
> { \
> T sum = (UT)x + (UT)IMM; \
> return (x ^ IMM) < 0 \
> ? sum \
> : (sum ^ x) >= 0 \
> ? sum \
> : x < 0 ? MIN : MAX; \
> }
>
> Passed the rv64gcv regression test.
>
> Signed-off-by: Li Xu <xu...@eswincomputing.com>
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/sat/sat_s_add_imm-2.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-1-i16.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-3.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-1-i32.c: ...
> * gcc.target/riscv/sat/sat_s_add_imm-4.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-1-i64.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-1.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-1-i8.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-run-2.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-run-1-i16.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-run-3.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-run-1-i32.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-run-4.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-run-1-i64.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-run-1.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm-run-1-i8.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-2-1.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm_type_check-1-i16.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-3-1.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm_type_check-1-i32.c: ...here.
> * gcc.target/riscv/sat/sat_s_add_imm-1-1.c: Move to...
> * gcc.target/riscv/sat/sat_s_add_imm_type_check-1-i8.c: ...here.
So for the tests, why are we forcing matching of the assembly code for
the entire function? That must makes for a fragile test as we may
change various aspects of code generation over time.
If the point of the patch is to detect SAT_ADD in more cases, then the
better and more stable test is to verify the existence of SAT_ADD the
appropriate number of times in the .optimized dump.
IMHO we really don't want this kind of whole function assembly matching.
Pan, do you have any further comments here? Do you have strong opinions
on whether or not we want to be doing this kind of assembly output
testing or not?
Jeff
