Thanks Jeff for comments, let me refine the comments in v2.
Pan
-----Original Message-----
From: Jeff Law <jeffreya...@gmail.com>
Sent: Sunday, August 4, 2024 6:25 AM
To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org
Cc: juzhe.zh...@rivai.ai; kito.ch...@gmail.com; rdapp....@gmail.com
Subject: Re: [PATCH v1] RISC-V: Support IMM for operand 0 of ussub pattern
On 8/3/24 3:33 AM, pan2...@intel.com wrote:
> From: Pan Li <pan2...@intel.com>
>
> This patch would like to allow IMM for the operand 0 of ussub pattern.
> Aka .SAT_SUB(1023, y) as the below example.
>
> Form 1:
> #define DEF_SAT_U_SUB_IMM_FMT_1(T, IMM) \
> T __attribute__((noinline)) \
> sat_u_sub_imm##IMM##_##T##_fmt_1 (T y) \
> { \
> return (T)IMM >= y ? (T)IMM - y : 0; \
> }
>
> DEF_SAT_U_SUB_IMM_FMT_1(uint64_t, 1023)
>
> Before this patch:
> 10 │ sat_u_sub_imm82_uint64_t_fmt_1:
> 11 │ li a5,82
> 12 │ bgtu a0,a5,.L3
> 13 │ sub a0,a5,a0
> 14 │ ret
> 15 │ .L3:
> 16 │ li a0,0
> 17 │ ret
>
> After this patch:
> 10 │ sat_u_sub_imm82_uint64_t_fmt_1:
> 11 │ li a5,82
> 12 │ sltu a4,a5,a0
> 13 │ addi a4,a4,-1
> 14 │ sub a0,a5,a0
> 15 │ and a0,a4,a0
> 16 │ ret
>
> The below test suites are passed for this patch:
> 1. The rv64gcv fully regression test.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv.cc (riscv_gen_unsigned_xmode_reg): Add new
> func impl to gen xmode rtx reg.
> (riscv_expand_ussub): Gen xmode reg for operand 1.
> * config/riscv/riscv.md: Allow const_int for operand 1.
> +> + 1. Case 1: .SAT_SUB (127, y) for QImode.
> + The imm will be (const_int 127) after expand_expr_real_1, thus we
> + can just move the (const_int 127) to Xmode reg without any other insn.
> +
> + 2. Case 2: .SAT_SUB (254, y) for QImode.
> + The imm will be (const_int -2) after expand_expr_real_1, thus we
> + will have li a0, -2 (aka a0 = 0xfffffffffffffffe if RV64). This is
> + not what we want for the underlying insn like sltu. So we need to
> + clean the up highest 56 bits for a0 to get the real value (254, 0xfe).
> +> + This function would like to take care of above scenario and
return the
> + rtx reg which holds the x in Xmode. */
What does this function do. ie, what are the inputs, what are the
outputs? Without that core information it's hard to know if your
implementation is correct.
If really looks like you're returning a reg in X mode. In which case
you can just gen_int_mode (constant, word_mode)
If the constant is 254, then that's going to load 0x00000000000000fe on
rv64.
If the problem is that you have a target of SImode on RV64, then you do
have a real problem. The rv64 ABI mandates that a 32bit value be sign
extended out to 64 bits. And if this is the problem you're trying to
solve, then it's a good indicator you've made a mistake elsewhere.
Anyway, it seems like you need to describe better where things are going
wrong before we can ACK/NACK this patch.
jeff
