diff --git a/gcc/testsuite/g++.dg/tree-ssa/phiprop-3.C b/gcc/testsuite/g++.dg/tree-ssa/phiprop-3.C new file mode 100644 index 00000000000..86ecb02b81b --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/phiprop-3.C @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */ + +/* PR tree-optimization/123120 */ +/* The order of min/max arguments should not cause any issues with phiprop, + the stores to the temps should not interfere with the loads from the temp. */ + +#include + +long f0(short c, short d, long long a) +{ + return std::max(std::min((long long)3, a), (long long)d); +} + +long f1(short c, short d, long long a) +{ + return std::max((long long)d, std::min((long long)3, a)); +} + +/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 4 "phiprop1"} } */ +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "release_ssa"} } */ +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "release_ssa"} } */ + diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-10.c b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-10.c new file mode 100644 index 00000000000..b630148faa5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-10.c @@ -0,0 +1,30 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra -fdump-tree-phiprop-details" } */ +/* PR tree-optimization/123120 */ +/* Make sure the store to *e conflicts with the store to *d */ + + +struct s1 +{ + int a; +}; + +void f(int *); + +void g(struct s1 i, struct s1 *d, struct s1 *e) +{ + const struct s1 t = {10}; + const struct s1 *a; + struct s1 t1 = {2}; + if (t.a < i.a) + a = &t; + else + a = &i; + *e = t1; + t1 = *a; + *d = t1; +} + +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop1"} } */ +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop2"} } */ + diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-11.c b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-11.c new file mode 100644 index 00000000000..6f078d175ef --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-11.c @@ -0,0 +1,30 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra -fdump-tree-phiprop-details" } */ +/* PR tree-optimization/123120 */ +/* Make sure the load *d conflicts with the store to *d so that phiprop + does not move the store into conditionals. */ + +struct s1 +{ + int a; +}; + +void f(int *); + +int g(struct s1 i, struct s1 *d, struct s1 *e) +{ + const struct s1 t = {10}; + const struct s1 *a; + struct s1 t1 = {2}; + if (t.a < i.a) + a = &t; + else + a = &i; + t1 = *d; + *d = *a; + return t1.a; +} + +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop1"} } */ +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop2"} } */ + diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-12.c b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-12.c new file mode 100644 index 00000000000..4cc2e5b8cd0 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-12.c @@ -0,0 +1,30 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-phiprop-details" } */ +/* PR tree-optimization/123120 */ +/* Make sure the load *d conflicts with the store to *d so that phiprop + does not move the store into conditionals. */ + +struct s1 +{ + int a; +}; + +void f(int *); + +int g(struct s1 i, struct s1 *d, struct s1 *e) +{ + const struct s1 t = {10}; + const struct s1 *a; + struct s1 t1 = {2}; + if (t.a < i.a) + a = &t; + else + a = &i; + int t2 = d->a; + *d = *a; + return t2; +} + +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop1"} } */ +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop2"} } */ + diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-8.c b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-8.c new file mode 100644 index 00000000000..546031e63d7 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-8.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */ + +/* PR tree-optimization/116823 */ +/* The clobber on b should not get in the way of phiprop here. */ +/* We should have MIN_EXPR early on. */ + +void f(int *); + +int g(int i) +{ + const int t = 10; + const int *a; + { + int b; + f(&b); + if (t < i) + a = &t; + else + a = &i; + } + return *a; +} + +/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 1 "phiprop1"} } */ +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "release_ssa"} } */ + diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-9.c b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-9.c new file mode 100644 index 00000000000..bbd91eda95d --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-9.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */ + +/* PR tree-optimization/123120 */ +/* The store to *d should not get in the way of phiprop here. */ +/* We should have MIN_EXPR early on. */ + +void f(int *); + +int g(int i, int *d) +{ + const int t = 10; + const int *a; + if (t < i) + a = &t; + else + a = &i; + *d = 1; + return *a; +} + +/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 1 "phiprop1"} } */ +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "release_ssa"} } */ + diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc index b7ce7b4a184..5985beae26a 100644 --- a/gcc/tree-ssa-phiprop.cc +++ b/gcc/tree-ssa-phiprop.cc @@ -101,12 +101,14 @@ struct phiprop_d }; /* Insert a new phi node for the dereference of PHI at basic_block - BB with the virtual operands from USE_STMT. */ + BB with the virtual operands from USE_STMT. The vuse for + the load will be set to OTHER_VUSE unless there is virtual op + phi for BB. */ static tree phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt, struct phiprop_d *phivn, size_t n, - bitmap dce_ssa_names) + bitmap dce_ssa_names, tree other_vuse) { tree res; gphi *new_phi = NULL; @@ -176,7 +178,7 @@ phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt, if (vphi) vuse = PHI_ARG_DEF_FROM_EDGE (vphi, e); else - vuse = gimple_vuse (use_stmt); + vuse = other_vuse; } else /* For the aggregate copy case updating virtual operands @@ -243,26 +245,85 @@ chk_uses (tree, tree *idx, void *data) incoming VUSE (up_vuse). If neither is present make sure the def stmt of the virtual use is in a different basic block dominating BB. When the def - is an edge-inserted one we know it dominates us. */ -static bool -can_handle_load (gimple *load_stmt, basic_block bb, - gphi *vphi, tree up_vuse) + is an edge-inserted one we know it dominates us. + Returns the vuse to use for the inserting. NULL_TREE + is returned when we can't do the insert. */ + +static tree +can_handle_load (gimple *load_stmt, + basic_block bb, + gphi *vphi, tree up_vuse, bool aggregate) { tree vuse = gimple_vuse (load_stmt); - if (vphi) - return vuse == gimple_phi_result (vphi); - if (up_vuse) - return vuse == up_vuse; - gimple *def_stmt = SSA_NAME_DEF_STMT (vuse); /* If the load does not have a store beforehand, then we can do the load in conditional. */ if (SSA_NAME_IS_DEFAULT_DEF (vuse)) - return true; - if (gimple_bb (def_stmt) != bb + { + /* For loads that have no stores before, there should be no + vphi. */ + gcc_checking_assert (!vphi); + /* The common vuse is the same as the default or there is none. */ + gcc_checking_assert (!up_vuse || up_vuse == vuse); + return vuse; + } + + /* If we have a vphi, then that needs to be end point. + If we have a common incoming vuse, that needs to be the end point. */ + tree expected_vuse = NULL_TREE; + if (vphi) + expected_vuse = gimple_phi_result (vphi); + else if (up_vuse) + expected_vuse = up_vuse; + + /* If the vuse on the load is the same as the expected vuse, + there are no stores inbetween. */ + if (vuse == expected_vuse) + return vuse; + /* Try to see if the store does not effect the load. */ + gimple *other_store = SSA_NAME_DEF_STMT (vuse); + /* Only handling the case where the store is in the same + bb as the phi. */ + if (gimple_bb (other_store) == bb) + { + /* For aggregates, skipping the store is too + hard to handle as you need to check for loads + and it is not worth the extra checks. */ + if (aggregate) + return NULL_TREE; + tree src = gimple_assign_rhs1 (load_stmt); + ao_ref read; + ao_ref_init (&read, src); + if (stmt_may_clobber_ref_p_1 (other_store, &read, false)) + return NULL_TREE; + vuse = gimple_vuse (other_store); + /* If that skipped store was the first store in program, + then we can do the load conditional. */ + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) + { + /* For loads that have no stores before, there should be no + vphi. */ + gcc_checking_assert (!vphi); + /* The common vuse is the same as the default or there is none. */ + gcc_checking_assert (!up_vuse || up_vuse == vuse); + return vuse; + } + other_store = SSA_NAME_DEF_STMT (vuse); + /* If the new vuse (after skipping) is the same as expected + then that is the vuse to return. */ + if (vuse == expected_vuse) + return vuse; + if (gimple_bb (other_store) == bb) + return NULL_TREE; + } + + /* If there was no an expected vuse then see if the vuse dominates the phi of + the address. */ + if (!expected_vuse && dominated_by_p (CDI_DOMINATORS, - bb, gimple_bb (def_stmt))) - return true; - return false; + bb, gimple_bb (other_store))) + return vuse; + + return NULL_TREE; } /* Propagate between the phi node arguments of PHI in BB and phi result @@ -385,13 +446,15 @@ propagate_with_phi (basic_block bb, gphi *vphi, gphi *phi, && !gimple_has_volatile_ops (use_stmt))) continue; - if (!can_handle_load (use_stmt, bb, vphi, up_vuse)) - continue; - bool aggregate = false; if (!is_gimple_reg_type (TREE_TYPE (gimple_assign_lhs (use_stmt)))) aggregate = true; + tree other_vuse; + other_vuse = can_handle_load (use_stmt, bb, vphi, up_vuse, aggregate); + if (!other_vuse) + continue; + if ((canpossible_trap || aggregate) && !dom_info_available_p (cfun, CDI_POST_DOMINATORS)) calculate_dominance_info (CDI_POST_DOMINATORS); @@ -459,7 +522,8 @@ propagate_with_phi (basic_block bb, gphi *vphi, gphi *phi, goto next; } - phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names); + phiprop_insert_phi (bb, phi, use_stmt, phivn, n, + dce_ssa_names, other_vuse); /* Remove old stmt. The phi and all of maybe its depedencies will be removed later via simple_dce_from_worklist. */ @@ -492,7 +556,8 @@ propagate_with_phi (basic_block bb, gphi *vphi, gphi *phi, else { tree vuse = gimple_vuse (use_stmt); - res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names); + res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n, + dce_ssa_names, other_vuse); type = TREE_TYPE (res); /* Remember the value we created for *ptr. */