phi-opt: Improve factor heurstic with constants and conversions from bool [PR116890]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
Take:
```
if (t_3(D) != 0)
goto <bb 3>;
else
goto <bb 4>;
<bb 3>
_8 = c_4(D) != 0;
_9 = (int) _8;
<bb 4>
# e_2 = PHI <_9(3), 0(2)>
```
We should factor out the conversion here as that will allow a simplfication to
`(t_3 != 0) & (c_4 != 0)`. Unlike most other types; `a ? b : CST` will simplify
for boolean result type to either `a | b` or `a & b` so allowing this conversion
for all operations will be always profitable.
Bootstrapped and tested on x86_64-linux-gnu with no regressions.
Note on the phi-opt-7.c testcase change, we are now able to optimize this
and remove the if due to the factoring out now so this is an improvement.
PR tree-optimization/116890
gcc/ChangeLog:
* tree-ssa-phiopt.cc (factor_out_conditional_operation): Conversions
from bool is also should be considered as wanting to happen.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/phi-opt-7.c: Update testcase for no ifs left.
* gcc.dg/tree-ssa/phi-opt-42.c: New test.
* gcc.dg/tree-ssa/phi-opt-43.c: New test.
Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c | 19 +++++++++++++++++++
gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c | 19 +++++++++++++++++++
gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c | 6 +++---
gcc/tree-ssa-phiopt.cc | 10 +++++++++-
4 files changed, 50 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c
Comments
On Mon, Sep 30, 2024 at 11:50 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> Take:
> ```
> if (t_3(D) != 0)
> goto <bb 3>;
> else
> goto <bb 4>;
>
> <bb 3>
> _8 = c_4(D) != 0;
> _9 = (int) _8;
>
> <bb 4>
> # e_2 = PHI <_9(3), 0(2)>
> ```
>
> We should factor out the conversion here as that will allow a simplfication to
> `(t_3 != 0) & (c_4 != 0)`. Unlike most other types; `a ? b : CST` will simplify
> for boolean result type to either `a | b` or `a & b` so allowing this conversion
> for all operations will be always profitable.
>
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> Note on the phi-opt-7.c testcase change, we are now able to optimize this
> and remove the if due to the factoring out now so this is an improvement.
OK.
Thanks,
Richard.
> PR tree-optimization/116890
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (factor_out_conditional_operation): Conversions
> from bool is also should be considered as wanting to happen.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/phi-opt-7.c: Update testcase for no ifs left.
> * gcc.dg/tree-ssa/phi-opt-42.c: New test.
> * gcc.dg/tree-ssa/phi-opt-43.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c | 19 +++++++++++++++++++
> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c | 19 +++++++++++++++++++
> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c | 6 +++---
> gcc/tree-ssa-phiopt.cc | 10 +++++++++-
> 4 files changed, 50 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c
> new file mode 100644
> index 00000000000..62556945159
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-optimized" } */
> +
> +/* PR tree-optimization/116890 */
> +
> +int f(int a, int b, int c)
> +{
> + int x;
> + if (c) x = a == 0;
> + else x = 0;
> + return x;
> +}
> +
> +
> +/* The if should have been removed as the the conversion from bool to int should have been factored out. */
> +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/
> +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. == 0" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */
> +
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c
> new file mode 100644
> index 00000000000..1d16f283f27
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-optimized" } */
> +
> +/* PR tree-optimization/116890 */
> +
> +int f(_Bool a, _Bool b, int c)
> +{
> + int x;
> + if (c) x = a & b;
> + else x = 0;
> + return x;
> +}
> +
> +
> +/* The if should have been removed as the the conversion from bool to int should have been factored out. */
> +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/
> +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. & b_\[0-9\]*.D." 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */
> +
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c
> index 51e1f6dfa75..3ee43e55692 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c
> @@ -15,8 +15,8 @@ int f(int t, int c)
> return g(d,e);
> }
>
> -/* There should be one ifs as one of them should be changed into
> - a conditional and the other should be there still. */
> -/* { dg-final { scan-tree-dump-times "if" 1 "optimized" } }*/
> +/* There should be no ifs as this is converted into `(t != 0) & (c != 0)`.
> +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/
> /* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = t_\[0-9\]*.D. != 0" 1 "optimized" } } */
>
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index d43832b390b..bd7f9607eb9 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -345,9 +345,17 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
> if (gassign *assign = dyn_cast <gassign *> (stmt))
> {
> tree lhs = gimple_assign_lhs (assign);
> + tree lhst = TREE_TYPE (lhs);
> enum tree_code ass_code
> = gimple_assign_rhs_code (assign);
> - if (ass_code != MAX_EXPR && ass_code != MIN_EXPR)
> + if (ass_code != MAX_EXPR && ass_code != MIN_EXPR
> + /* Conversions from boolean like types is ok
> + as `a?1:b` and `a?0:b` will always simplify
> + to `a & b` or `a | b`.
> + See PR 116890. */
> + && !(INTEGRAL_TYPE_P (lhst)
> + && TYPE_UNSIGNED (lhst)
> + && TYPE_PRECISION (lhst) == 1))
> return NULL;
> if (lhs != gimple_assign_rhs1 (arg0_def_stmt))
> return NULL;
> --
> 2.34.1
>
new file mode 100644
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+/* PR tree-optimization/116890 */
+
+int f(int a, int b, int c)
+{
+ int x;
+ if (c) x = a == 0;
+ else x = 0;
+ return x;
+}
+
+
+/* The if should have been removed as the the conversion from bool to int should have been factored out. */
+/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/
+/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. == 0" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */
+
new file mode 100644
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+/* PR tree-optimization/116890 */
+
+int f(_Bool a, _Bool b, int c)
+{
+ int x;
+ if (c) x = a & b;
+ else x = 0;
+ return x;
+}
+
+
+/* The if should have been removed as the the conversion from bool to int should have been factored out. */
+/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/
+/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. & b_\[0-9\]*.D." 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */
+
@@ -15,8 +15,8 @@ int f(int t, int c)
return g(d,e);
}
-/* There should be one ifs as one of them should be changed into
- a conditional and the other should be there still. */
-/* { dg-final { scan-tree-dump-times "if" 1 "optimized" } }*/
+/* There should be no ifs as this is converted into `(t != 0) & (c != 0)`.
+/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/
/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = t_\[0-9\]*.D. != 0" 1 "optimized" } } */
@@ -345,9 +345,17 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
if (gassign *assign = dyn_cast <gassign *> (stmt))
{
tree lhs = gimple_assign_lhs (assign);
+ tree lhst = TREE_TYPE (lhs);
enum tree_code ass_code
= gimple_assign_rhs_code (assign);
- if (ass_code != MAX_EXPR && ass_code != MIN_EXPR)
+ if (ass_code != MAX_EXPR && ass_code != MIN_EXPR
+ /* Conversions from boolean like types is ok
+ as `a?1:b` and `a?0:b` will always simplify
+ to `a & b` or `a | b`.
+ See PR 116890. */
+ && !(INTEGRAL_TYPE_P (lhst)
+ && TYPE_UNSIGNED (lhst)
+ && TYPE_PRECISION (lhst) == 1))
return NULL;
if (lhs != gimple_assign_rhs1 (arg0_def_stmt))
return NULL;