* [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization
@ 2023-11-23 5:47 Feng Wang
2023-11-23 6:34 ` Andrew Pinski
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Feng Wang @ 2023-11-23 5:47 UTC (permalink / raw)
To: gcc-patches; +Cc: kito.cheng, jeffreyalaw, Feng Wang
This patch add another condition for gimple-cond optimization. Refer to
the following test case.
int foo1 (int data, int res)
{
res = data & 0xf;
res |= res << 4;
if (res < 0x22)
return 0x22;
return res;
}
with the compilation flag "-march=rv64gc_zba_zbb -mabi=lp64d -O2",
before this patch the compilation result is
foo1:
andi a0,a0,15
slliw a5,a0,4
addw a3,a5,a0
li a4,33
add a0,a5,a0
bleu a3,a4,.L5
ret
.L5:
li a0,34
ret
after this patch the compilation result is
foo1:
andi a0,a0,15
slliw a5,a0,4
add a5,a5,a0
li a0,34
max a0,a5,a0
ret
The reason is in the pass_early_vrp, the arg0 of gimple_cond
is replaced,but the PHI node still use the arg0.
The some of evrp pass logs are as follows
gimple_assign <mult_expr, _9, _7, 17, NULL>
gimple_assign <nop_expr, res_5, _9, NULL, NULL>
gimple_cond <le_expr, _9, 33, NULL, NULL>
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
<bb 3> :
// predicted unlikely by early return (on trees) predictor.
<bb 4> :
# gimple_phi <_2, 34(3), res_5(2)>
The arg0 of gimple_cond is replaced by _9,but the gimple_phi still
uses res_5,it will cause optimization fail of PHI node to MAX_EXPR.
So the next_use_is_phi is added to control the replacement.
gcc/ChangeLog:
* vr-values.cc (next_use_is_phi):
(simplify_using_ranges::simplify_casted_compare):
add new function next_use_is_phi to control the replacement.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zbb-min-max-04.c: New test.
---
gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c | 14 ++++++++++++++
gcc/vr-values.cc | 15 ++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
new file mode 100644
index 00000000000..8c3e87a35e0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zba_zbb -mabi=lp64d" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
+
+int foo1 (int data, int res)
+{
+ res = data & 0xf;
+ res |= res << 4;
+ if (res < 0x22)
+ return 0x22;
+ return res;
+}
+
+/* { dg-final { scan-assembler-times "max\t" 1 } } */
\ No newline at end of file
diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
index ecb294131b0..1f7a727c638 100644
--- a/gcc/vr-values.cc
+++ b/gcc/vr-values.cc
@@ -1263,6 +1263,18 @@ simplify_using_ranges::simplify_compare_using_ranges_1 (tree_code &cond_code, tr
return happened;
}
+/* Return true if the next use of SSA_NAME is PHI node */
+bool
+next_use_is_phi (tree arg)
+{
+ use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (arg));
+ use_operand_p next = imm->next;
+ if (next && next->loc.stmt
+ && (gimple_code (next->loc.stmt) == GIMPLE_PHI))
+ return true;
+ return false;
+}
+
/* Simplify OP0 code OP1 when OP1 is a constant and OP0 was a SSA_NAME
defined by a type conversion. Replacing OP0 with RHS of the type conversion.
Doing so makes the conversion dead which helps subsequent passes. */
@@ -1305,7 +1317,8 @@ simplify_using_ranges::simplify_casted_compare (tree_code &, tree &op0, tree &op
if (TREE_CODE (innerop) == SSA_NAME
&& !POINTER_TYPE_P (TREE_TYPE (innerop))
&& !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (innerop)
- && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0)))
+ && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0))
+ && !next_use_is_phi (op0))
{
value_range vr;
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization
2023-11-23 5:47 [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization Feng Wang
@ 2023-11-23 6:34 ` Andrew Pinski
2023-11-23 8:17 ` Feng Wang
2023-11-23 6:36 ` Andrew Pinski
2024-05-26 14:40 ` Jeff Law
2 siblings, 1 reply; 5+ messages in thread
From: Andrew Pinski @ 2023-11-23 6:34 UTC (permalink / raw)
To: Feng Wang; +Cc: gcc-patches, kito.cheng, jeffreyalaw
On Wed, Nov 22, 2023 at 10:07 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
>
> This patch add another condition for gimple-cond optimization. Refer to
> the following test case.
> int foo1 (int data, int res)
> {
> res = data & 0xf;
> res |= res << 4;
> if (res < 0x22)
> return 0x22;
> return res;
> }
> with the compilation flag "-march=rv64gc_zba_zbb -mabi=lp64d -O2",
> before this patch the compilation result is
> foo1:
> andi a0,a0,15
> slliw a5,a0,4
> addw a3,a5,a0
> li a4,33
> add a0,a5,a0
> bleu a3,a4,.L5
> ret
> .L5:
> li a0,34
> ret
> after this patch the compilation result is
> foo1:
> andi a0,a0,15
> slliw a5,a0,4
> add a5,a5,a0
> li a0,34
> max a0,a5,a0
> ret
> The reason is in the pass_early_vrp, the arg0 of gimple_cond
> is replaced,but the PHI node still use the arg0.
> The some of evrp pass logs are as follows
> gimple_assign <mult_expr, _9, _7, 17, NULL>
> gimple_assign <nop_expr, res_5, _9, NULL, NULL>
> gimple_cond <le_expr, _9, 33, NULL, NULL>
> goto <bb 3>; [INV]
> else
> goto <bb 4>; [INV]
>
> <bb 3> :
> // predicted unlikely by early return (on trees) predictor.
>
> <bb 4> :
> # gimple_phi <_2, 34(3), res_5(2)>
> The arg0 of gimple_cond is replaced by _9,but the gimple_phi still
> uses res_5,it will cause optimization fail of PHI node to MAX_EXPR.
> So the next_use_is_phi is added to control the replacement.
I don't think this is the correct appoarch here.
We end up with the same original issue if we had wrote it like:
```
int foo1 (int data, int res)
{
res = data & 0xf;
unsigned int r = res;
r*=17;
res = r;
if (r < 0x22)
return 0x22;
return res;
}
```
I suspect instead we should extend the match.pd patterns to match this max.
We should be able to extend:
```
(for cmp (lt le gt ge eq ne)
(simplify
(cond (cmp (convert1? @1) INTEGER_CST@3) (convert2? @1) INTEGER_CST@2)
(with
```
To match instead by changing the second @1 with @4 and then using
bitwise_equal_p . If @1 != @4 but bitwise_equal_p is true, you need to
make sure the outer convert1/convert2 are nop conversions so that you
get the same extension I think ...
Note you could instead improve minmax_replacement but I have been in
the process of moving those changes to match.pd.
Thanks,
Andrew Pinski
>
> gcc/ChangeLog:
>
> * vr-values.cc (next_use_is_phi):
> (simplify_using_ranges::simplify_casted_compare):
> add new function next_use_is_phi to control the replacement.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/zbb-min-max-04.c: New test.
> ---
> gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c | 14 ++++++++++++++
> gcc/vr-values.cc | 15 ++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
>
> diff --git a/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
> new file mode 100644
> index 00000000000..8c3e87a35e0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_zba_zbb -mabi=lp64d" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> +
> +int foo1 (int data, int res)
> +{
> + res = data & 0xf;
> + res |= res << 4;
> + if (res < 0x22)
> + return 0x22;
> + return res;
> +}
> +
> +/* { dg-final { scan-assembler-times "max\t" 1 } } */
> \ No newline at end of file
> diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
> index ecb294131b0..1f7a727c638 100644
> --- a/gcc/vr-values.cc
> +++ b/gcc/vr-values.cc
> @@ -1263,6 +1263,18 @@ simplify_using_ranges::simplify_compare_using_ranges_1 (tree_code &cond_code, tr
> return happened;
> }
>
> +/* Return true if the next use of SSA_NAME is PHI node */
> +bool
> +next_use_is_phi (tree arg)
> +{
> + use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (arg));
> + use_operand_p next = imm->next;
> + if (next && next->loc.stmt
> + && (gimple_code (next->loc.stmt) == GIMPLE_PHI))
> + return true;
> + return false;
> +}
> +
> /* Simplify OP0 code OP1 when OP1 is a constant and OP0 was a SSA_NAME
> defined by a type conversion. Replacing OP0 with RHS of the type conversion.
> Doing so makes the conversion dead which helps subsequent passes. */
> @@ -1305,7 +1317,8 @@ simplify_using_ranges::simplify_casted_compare (tree_code &, tree &op0, tree &op
> if (TREE_CODE (innerop) == SSA_NAME
> && !POINTER_TYPE_P (TREE_TYPE (innerop))
> && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (innerop)
> - && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0)))
> + && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0))
> + && !next_use_is_phi (op0))
> {
> value_range vr;
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization
2023-11-23 5:47 [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization Feng Wang
2023-11-23 6:34 ` Andrew Pinski
@ 2023-11-23 6:36 ` Andrew Pinski
2024-05-26 14:40 ` Jeff Law
2 siblings, 0 replies; 5+ messages in thread
From: Andrew Pinski @ 2023-11-23 6:36 UTC (permalink / raw)
To: Feng Wang; +Cc: gcc-patches, kito.cheng, jeffreyalaw
On Wed, Nov 22, 2023 at 10:07 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
>
> This patch add another condition for gimple-cond optimization. Refer to
> the following test case.
> int foo1 (int data, int res)
> {
> res = data & 0xf;
> res |= res << 4;
> if (res < 0x22)
> return 0x22;
> return res;
> }
> with the compilation flag "-march=rv64gc_zba_zbb -mabi=lp64d -O2",
> before this patch the compilation result is
> foo1:
> andi a0,a0,15
> slliw a5,a0,4
> addw a3,a5,a0
> li a4,33
> add a0,a5,a0
> bleu a3,a4,.L5
> ret
> .L5:
> li a0,34
> ret
> after this patch the compilation result is
> foo1:
> andi a0,a0,15
> slliw a5,a0,4
> add a5,a5,a0
> li a0,34
> max a0,a5,a0
> ret
> The reason is in the pass_early_vrp, the arg0 of gimple_cond
> is replaced,but the PHI node still use the arg0.
> The some of evrp pass logs are as follows
> gimple_assign <mult_expr, _9, _7, 17, NULL>
> gimple_assign <nop_expr, res_5, _9, NULL, NULL>
> gimple_cond <le_expr, _9, 33, NULL, NULL>
> goto <bb 3>; [INV]
> else
> goto <bb 4>; [INV]
>
> <bb 3> :
> // predicted unlikely by early return (on trees) predictor.
>
> <bb 4> :
> # gimple_phi <_2, 34(3), res_5(2)>
> The arg0 of gimple_cond is replaced by _9,but the gimple_phi still
> uses res_5,it will cause optimization fail of PHI node to MAX_EXPR.
> So the next_use_is_phi is added to control the replacement.
>
> gcc/ChangeLog:
>
> * vr-values.cc (next_use_is_phi):
> (simplify_using_ranges::simplify_casted_compare):
> add new function next_use_is_phi to control the replacement.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/zbb-min-max-04.c: New test.
One more comment, since this is a generic gimple change, you should
add a testcase that is not riscv specific that scans the tree dumps. I
would scan phiopt1 in this case to make sure we MAX_EXPR is created
early on.
Thanks,
Andrew Pinski
> ---
> gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c | 14 ++++++++++++++
> gcc/vr-values.cc | 15 ++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
>
> diff --git a/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
> new file mode 100644
> index 00000000000..8c3e87a35e0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_zba_zbb -mabi=lp64d" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> +
> +int foo1 (int data, int res)
> +{
> + res = data & 0xf;
> + res |= res << 4;
> + if (res < 0x22)
> + return 0x22;
> + return res;
> +}
> +
> +/* { dg-final { scan-assembler-times "max\t" 1 } } */
> \ No newline at end of file
> diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
> index ecb294131b0..1f7a727c638 100644
> --- a/gcc/vr-values.cc
> +++ b/gcc/vr-values.cc
> @@ -1263,6 +1263,18 @@ simplify_using_ranges::simplify_compare_using_ranges_1 (tree_code &cond_code, tr
> return happened;
> }
>
> +/* Return true if the next use of SSA_NAME is PHI node */
> +bool
> +next_use_is_phi (tree arg)
> +{
> + use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (arg));
> + use_operand_p next = imm->next;
> + if (next && next->loc.stmt
> + && (gimple_code (next->loc.stmt) == GIMPLE_PHI))
> + return true;
> + return false;
> +}
> +
> /* Simplify OP0 code OP1 when OP1 is a constant and OP0 was a SSA_NAME
> defined by a type conversion. Replacing OP0 with RHS of the type conversion.
> Doing so makes the conversion dead which helps subsequent passes. */
> @@ -1305,7 +1317,8 @@ simplify_using_ranges::simplify_casted_compare (tree_code &, tree &op0, tree &op
> if (TREE_CODE (innerop) == SSA_NAME
> && !POINTER_TYPE_P (TREE_TYPE (innerop))
> && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (innerop)
> - && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0)))
> + && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0))
> + && !next_use_is_phi (op0))
> {
> value_range vr;
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization
2023-11-23 6:34 ` Andrew Pinski
@ 2023-11-23 8:17 ` Feng Wang
0 siblings, 0 replies; 5+ messages in thread
From: Feng Wang @ 2023-11-23 8:17 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches, kito.cheng, Jeff Law
On 2023-11-23 14:34 Andrew Pinski<pinskia@gmail.com> wrote:
>
>On Wed, Nov 22, 2023 at 10:07 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
>>
>> This patch add another condition for gimple-cond optimization. Refer to
>> the following test case.
>> int foo1 (int data, int res)
>> {
>> res = data & 0xf;
>> res |= res << 4;
>> if (res < 0x22)
>> return 0x22;
>> return res;
>> }
>> with the compilation flag "-march=rv64gc_zba_zbb -mabi=lp64d -O2",
>> before this patch the compilation result is
>> foo1:
>> andi a0,a0,15
>> slliw a5,a0,4
>> addw a3,a5,a0
>> li a4,33
>> add a0,a5,a0
>> bleu a3,a4,.L5
>> ret
>> .L5:
>> li a0,34
>> ret
>> after this patch the compilation result is
>> foo1:
>> andi a0,a0,15
>> slliw a5,a0,4
>> add a5,a5,a0
>> li a0,34
>> max a0,a5,a0
>> ret
>> The reason is in the pass_early_vrp, the arg0 of gimple_cond
>> is replaced,but the PHI node still use the arg0.
>> The some of evrp pass logs are as follows
>> gimple_assign <mult_expr, _9, _7, 17, NULL>
>> gimple_assign <nop_expr, res_5, _9, NULL, NULL>
>> gimple_cond <le_expr, _9, 33, NULL, NULL>
>> goto <bb 3>; [INV]
>> else
>> goto <bb 4>; [INV]
>>
>> <bb 3> :
>> // predicted unlikely by early return (on trees) predictor.
>>
>> <bb 4> :
>> # gimple_phi <_2, 34(3), res_5(2)>
>> The arg0 of gimple_cond is replaced by _9,but the gimple_phi still
>> uses res_5,it will cause optimization fail of PHI node to MAX_EXPR.
>> So the next_use_is_phi is added to control the replacement.
>
>I don't think this is the correct appoarch here.
>We end up with the same original issue if we had wrote it like:
>```
>int foo1 (int data, int res)
>{
> res = data & 0xf;
> unsigned int r = res;
> r*=17;
> res = r;
> if (r < 0x22)
> return 0x22;
> return res;
>}
>```
>I suspect instead we should extend the match.pd patterns to match this max.
>We should be able to extend:
>```
>(for cmp (lt le gt ge eq ne)
> (simplify
> (cond (cmp (convert1? @1) INTEGER_CST@3) (convert2? @1) INTEGER_CST@2)
> (with
>```
>To match instead by changing the second @1 with @4 and then using
>bitwise_equal_p . If @1 != @4 but bitwise_equal_p is true, you need to
>make sure the outer convert1/convert2 are nop conversions so that you
>get the same extension I think ...
>
>Note you could instead improve minmax_replacement but I have been in
>the process of moving those changes to match.pd.
>
>Thanks,
>Andrew Pinski
Thanks for your feedback. The minmax replacement happens in phiopt pass, there is one condition
that requires the "arg_false"(from PHI node) should be same with "smaller"(from gimple_cond).
So I made this change. But as you said, this modification is not very suitable, and I have not considered
it comprehensively. I'm not very familiar with match.pd, can it solve this judgment problem?
Thanks,
Feng Wang
>
>>
>> gcc/ChangeLog:
>>
>> * vr-values.cc (next_use_is_phi):
>> (simplify_using_ranges::simplify_casted_compare):
>> add new function next_use_is_phi to control the replacement.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/zbb-min-max-04.c: New test.
>> ---
>> gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c | 14 ++++++++++++++
>> gcc/vr-values.cc | 15 ++++++++++++++-
>> 2 files changed, 28 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
>>
>> diff --git a/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
>> new file mode 100644
>> index 00000000000..8c3e87a35e0
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
>> @@ -0,0 +1,14 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-march=rv64gc_zba_zbb -mabi=lp64d" } */
>> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
>> +
>> +int foo1 (int data, int res)
>> +{
>> + res = data & 0xf;
>> + res |= res << 4;
>> + if (res < 0x22)
>> + return 0x22;
>> + return res;
>> +}
>> +
>> +/* { dg-final { scan-assembler-times "max\t" 1 } } */
>> \ No newline at end of file
>> diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
>> index ecb294131b0..1f7a727c638 100644
>> --- a/gcc/vr-values.cc
>> +++ b/gcc/vr-values.cc
>> @@ -1263,6 +1263,18 @@ simplify_using_ranges::simplify_compare_using_ranges_1 (tree_code &cond_code, tr
>> return happened;
>> }
>>
>> +/* Return true if the next use of SSA_NAME is PHI node */
>> +bool
>> +next_use_is_phi (tree arg)
>> +{
>> + use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (arg));
>> + use_operand_p next = imm->next;
>> + if (next && next->loc.stmt
>> + && (gimple_code (next->loc.stmt) == GIMPLE_PHI))
>> + return true;
>> + return false;
>> +}
>> +
>> /* Simplify OP0 code OP1 when OP1 is a constant and OP0 was a SSA_NAME
>> defined by a type conversion. Replacing OP0 with RHS of the type conversion.
>> Doing so makes the conversion dead which helps subsequent passes. */
>> @@ -1305,7 +1317,8 @@ simplify_using_ranges::simplify_casted_compare (tree_code &, tree &op0, tree &op
>> if (TREE_CODE (innerop) == SSA_NAME
>> && !POINTER_TYPE_P (TREE_TYPE (innerop))
>> && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (innerop)
>> - && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0)))
>> + && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0))
>> + && !next_use_is_phi (op0))
>> {
>> value_range vr;
>>
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization
2023-11-23 5:47 [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization Feng Wang
2023-11-23 6:34 ` Andrew Pinski
2023-11-23 6:36 ` Andrew Pinski
@ 2024-05-26 14:40 ` Jeff Law
2 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2024-05-26 14:40 UTC (permalink / raw)
To: Feng Wang, gcc-patches; +Cc: kito.cheng
On 11/22/23 10:47 PM, Feng Wang wrote:
> This patch add another condition for gimple-cond optimization. Refer to
> the following test case.
> int foo1 (int data, int res)
> {
> res = data & 0xf;
> res |= res << 4;
> if (res < 0x22)
> return 0x22;
> return res;
> }
> with the compilation flag "-march=rv64gc_zba_zbb -mabi=lp64d -O2",
> before this patch the compilation result is
> foo1:
> andi a0,a0,15
> slliw a5,a0,4
> addw a3,a5,a0
> li a4,33
> add a0,a5,a0
> bleu a3,a4,.L5
> ret
> .L5:
> li a0,34
> ret
> after this patch the compilation result is
> foo1:
> andi a0,a0,15
> slliw a5,a0,4
> add a5,a5,a0
> li a0,34
> max a0,a5,a0
> ret
> The reason is in the pass_early_vrp, the arg0 of gimple_cond
> is replaced,but the PHI node still use the arg0.
> The some of evrp pass logs are as follows
> gimple_assign <mult_expr, _9, _7, 17, NULL>
> gimple_assign <nop_expr, res_5, _9, NULL, NULL>
> gimple_cond <le_expr, _9, 33, NULL, NULL>
> goto <bb 3>; [INV]
> else
> goto <bb 4>; [INV]
>
> <bb 3> :
> // predicted unlikely by early return (on trees) predictor.
>
> <bb 4> :
> # gimple_phi <_2, 34(3), res_5(2)>
> The arg0 of gimple_cond is replaced by _9,but the gimple_phi still
> uses res_5,it will cause optimization fail of PHI node to MAX_EXPR.
> So the next_use_is_phi is added to control the replacement.
>
> gcc/ChangeLog:
>
> * vr-values.cc (next_use_is_phi):
> (simplify_using_ranges::simplify_casted_compare):
> add new function next_use_is_phi to control the replacement.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/zbb-min-max-04.c: New test.
I'm not sure what changed, but AFAICT this patch isn't needed anymore.
We get the desired code with the trunk compiler.
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-26 14:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-23 5:47 [PATCH] gimple-vr-values:Add constraint for gimple-cond optimization Feng Wang
2023-11-23 6:34 ` Andrew Pinski
2023-11-23 8:17 ` Feng Wang
2023-11-23 6:36 ` Andrew Pinski
2024-05-26 14:40 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).