Re: Re: [PATCH 5/5] [ifcvt] optimize extension for x=c ? (y op z) : y by RISC-V Zicond like insns
On 2023-12-11 13:46 Jeff Law wrote:
>
>
>
>On 12/5/23 01:12, Fei Gao wrote:
>> SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered
>> to support SImode in 64-bit machine.
>>
>> Co-authored-by: Xiao Zeng
>>
>> gcc/ChangeLog:
>>
>> * ifcvt.cc (noce_cond_zero_binary_op_supported): add support for extension
>> (noce_bbs_ok_for_cond_zero_arith): likewise
>> (noce_try_cond_zero_arith): support extension of LSHIFTRT case
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/zicond_ifcvt_opt.c: add TCs for extension
>So I think this needs to defer to gcc-15. But even so I think getting
>some review on the effort is useful.
>
>
>> ---
>> gcc/ifcvt.cc | 16 ++-
>> .../gcc.target/riscv/zicond_ifcvt_opt.c | 126 +-
>> 2 files changed, 139 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
>> index b84be53ec5c..306497a8e37 100644
>> --- a/gcc/ifcvt.cc
>> +++ b/gcc/ifcvt.cc
>> @@ -2934,6 +2934,10 @@ noce_cond_zero_binary_op_supported (rtx op)
>> {
>> enum rtx_code opcode = GET_CODE (op);
>>
>> + /* Strip SIGN_EXTEND or ZERO_EXTEND if any. */
>> + if (opcode == SIGN_EXTEND || opcode == ZERO_EXTEND)
>> + opcode = GET_CODE (XEXP (op, 0));
>So it seems to me like that you need to record what the extension was so
>that you can re-apply it to the result.
>
>> @@ -3114,7 +3122,11 @@ noce_try_cond_zero_arith (struct noce_if_info
>> *if_info)
>> if (CONST_INT_P (*to_replace))
>> {
>> if (noce_cond_zero_shift_op_supported (bin_code))
>> - *to_replace = gen_rtx_SUBREG (E_QImode, target, 0);
>> + {
>> + *to_replace = gen_rtx_SUBREG (E_QImode, target, 0);
>> + if (GET_CODE (a) == ZERO_EXTEND && bin_code == LSHIFTRT)
>> +PUT_CODE (a, SIGN_EXTEND);
>> + }
>This doesn't look correct (ignoring the SUBREG issues with patch #4 in
>this series).
Agree there's issue here for const_int case as you mentioned in
[PATCH 4/5] [ifcvt] optimize x=c ? (y op const_int) : y.
>
>When we looked at this internally the conclusion was we needed to first
>strip the extension, recording what kind of extension it was, then
>reapply the same extension to the result of the now conditional
>operation. And it's independent of SUBREG handling.
Ignoring the const_int case, we can reuse the RTL pattern and replace
the z(SUBREG pr REG) in INSN_A(x=y op z) without recording what kind
of extension it was.
New patch will be sent to gcc15.
BR,
Fei
>
>
>Jeff
Re: [PATCH 5/5] [ifcvt] optimize extension for x=c ? (y op z) : y by RISC-V Zicond like insns
On 12/5/23 01:12, Fei Gao wrote:
SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered
to support SImode in 64-bit machine.
Co-authored-by: Xiao Zeng
gcc/ChangeLog:
* ifcvt.cc (noce_cond_zero_binary_op_supported): add support for
extension
(noce_bbs_ok_for_cond_zero_arith): likewise
(noce_try_cond_zero_arith): support extension of LSHIFTRT case
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zicond_ifcvt_opt.c: add TCs for extension
So I think this needs to defer to gcc-15. But even so I think getting
some review on the effort is useful.
---
gcc/ifcvt.cc | 16 ++-
.../gcc.target/riscv/zicond_ifcvt_opt.c | 126 +-
2 files changed, 139 insertions(+), 3 deletions(-)
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index b84be53ec5c..306497a8e37 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -2934,6 +2934,10 @@ noce_cond_zero_binary_op_supported (rtx op)
{
enum rtx_code opcode = GET_CODE (op);
+ /* Strip SIGN_EXTEND or ZERO_EXTEND if any. */
+ if (opcode == SIGN_EXTEND || opcode == ZERO_EXTEND)
+opcode = GET_CODE (XEXP (op, 0));
So it seems to me like that you need to record what the extension was so
that you can re-apply it to the result.
@@ -3114,7 +3122,11 @@ noce_try_cond_zero_arith (struct noce_if_info *if_info)
if (CONST_INT_P (*to_replace))
{
if (noce_cond_zero_shift_op_supported (bin_code))
- *to_replace = gen_rtx_SUBREG (E_QImode, target, 0);
+ {
+ *to_replace = gen_rtx_SUBREG (E_QImode, target, 0);
+ if (GET_CODE (a) == ZERO_EXTEND && bin_code == LSHIFTRT)
+ PUT_CODE (a, SIGN_EXTEND);
+ }
This doesn't look correct (ignoring the SUBREG issues with patch #4 in
this series).
When we looked at this internally the conclusion was we needed to first
strip the extension, recording what kind of extension it was, then
reapply the same extension to the result of the now conditional
operation. And it's independent of SUBREG handling.
Jeff
[PATCH 5/5] [ifcvt] optimize extension for x=c ? (y op z) : y by RISC-V Zicond like insns
SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered
to support SImode in 64-bit machine.
Co-authored-by: Xiao Zeng
gcc/ChangeLog:
* ifcvt.cc (noce_cond_zero_binary_op_supported): add support for
extension
(noce_bbs_ok_for_cond_zero_arith): likewise
(noce_try_cond_zero_arith): support extension of LSHIFTRT case
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zicond_ifcvt_opt.c: add TCs for extension
---
gcc/ifcvt.cc | 16 ++-
.../gcc.target/riscv/zicond_ifcvt_opt.c | 126 +-
2 files changed, 139 insertions(+), 3 deletions(-)
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index b84be53ec5c..306497a8e37 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -2934,6 +2934,10 @@ noce_cond_zero_binary_op_supported (rtx op)
{
enum rtx_code opcode = GET_CODE (op);
+ /* Strip SIGN_EXTEND or ZERO_EXTEND if any. */
+ if (opcode == SIGN_EXTEND || opcode == ZERO_EXTEND)
+opcode = GET_CODE (XEXP (op, 0));
+
if (opcode == PLUS || opcode == MINUS || opcode == IOR || opcode == XOR
|| opcode == AND || noce_cond_zero_shift_op_supported (opcode))
return true;
@@ -3000,7 +3004,11 @@ noce_bbs_ok_for_cond_zero_arith (struct noce_if_info
*if_info, rtx *common_ptr,
if (!(noce_cond_zero_binary_op_supported (a) && REG_P (b)))
return false;
- bin_exp = a;
+ /* Strip sign_extend if any. */
+ if (GET_CODE (a) == SIGN_EXTEND || GET_CODE (a) == ZERO_EXTEND)
+bin_exp = XEXP (a, 0);
+ else
+bin_exp = a;
/* Canonicalize x = (z op y) : y to x = (y op z) : y */
op1 = get_base_reg (XEXP (bin_exp, 1));
@@ -3114,7 +3122,11 @@ noce_try_cond_zero_arith (struct noce_if_info *if_info)
if (CONST_INT_P (*to_replace))
{
if (noce_cond_zero_shift_op_supported (bin_code))
- *to_replace = gen_rtx_SUBREG (E_QImode, target, 0);
+ {
+ *to_replace = gen_rtx_SUBREG (E_QImode, target, 0);
+ if (GET_CODE (a) == ZERO_EXTEND && bin_code == LSHIFTRT)
+ PUT_CODE (a, SIGN_EXTEND);
+ }
else if (SUBREG_P (bin_op0))
*to_replace = gen_rtx_SUBREG (GET_MODE (bin_op0), target, 0);
else
diff --git a/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c
b/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c
index 85743e1734c..53206d76e9f 100644
--- a/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c
+++ b/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c
@@ -615,6 +615,69 @@ test_RotateR_eqz (unsigned long x, unsigned long y,
unsigned long z,
return x;
}
+int
+test_ADD_ceqz_int (int x, int y, int z, int c)
+{
+ if (c)
+x = y + z;
+ else
+x = y;
+ return x;
+}
+
+int
+test_ShiftLeft_eqz_int (int x, int y, int z, int c)
+{
+ if (c)
+x = y << z;
+ else
+x = y;
+ return x;
+}
+
+int
+test_ShiftR_eqz_int (int x, int y, int z, int c)
+{
+ if (c)
+x = y >> z;
+ else
+x = y;
+ return x;
+}
+
+unsigned int
+test_ShiftR_logical_eqz_int (unsigned int x, unsigned int y, unsigned int z,
+unsigned int c)
+{
+ if (c)
+x = y >> z;
+ else
+x = y;
+ return x;
+}
+
+unsigned int
+test_RotateL_eqz_int (unsigned int x, unsigned int y, unsigned int z,
+ unsigned int c)
+{
+ if (c)
+x = (y << z) | (y >> (32 - z));
+ else
+x = y;
+ return x;
+}
+
+unsigned int
+test_RotateR_eqz_int (unsigned int x, unsigned int y, unsigned int z,
+ unsigned int c)
+{
+ if (c)
+x = (y >> z) | (y << (32 - z));
+ else
+x = y;
+ return x;
+}
+
long
test_ADD_ceqz_imm (long x, long y, long c)
{
@@ -1225,6 +1288,67 @@ test_RotateR_eqz_imm (unsigned long x, unsigned long y,
unsigned long c)
x = y;
return x;
}
+
+int
+test_ADD_ceqz_imm_int (int x, int y, int c)
+{
+ if (c)
+x = y + 11;
+ else
+x = y;
+ return x;
+}
+
+int
+test_ShiftLeft_eqz_imm_int (int x, int y, int c)
+{
+ if (c)
+x = y << 11;
+ else
+x = y;
+ return x;
+}
+
+int
+test_ShiftR_eqz_imm_int (int x, int y, int c)
+{
+ if (c)
+x = y >> 11;
+ else
+x = y;
+ return x;
+}
+
+unsigned int
+test_ShiftR_logical_eqz_imm_int (unsigned int x, unsigned int y, unsigned int
c)
+{
+ if (c)
+x = y >> 11;
+ else
+x = y;
+ return x;
+}
+
+unsigned int
+test_RotateL_eqz_imm_int (unsigned int x, unsigned int y, unsigned int c)
+{
+ if (c)
+x = (y << 11) | (y >> (32 - 11));
+ else
+x = y;
+ return x;
+}
+
+unsigned int
+test_RotateR_eqz_imm_int (unsigned int x, unsigned int y, unsigned int c)
+{
+ if (c)
+x = (y >> 11) | (y << (32 - 11));
+ else
+x = y;
+ return x;
+}
+
long
test_AND_ceqz (long x, long y, long z, long c)
{
@@ -1544,5 +1668,5 @@ test_AND_eqz_x_2_imm_reverse_bin_oprands (long x, long c)
x = 11 & x;
return x;
}
-/* { dg-final { scan-assembler-times {czero\.eqz} 82 } } */
+/* { dg-final { scan-assembler-times {czero\.eqz} 94 } } */
/* { dg-final { scan-assembler-times
