PHIOPT: Improve readability of tree_ssa_phiopt_worker
Checks
Commit Message
This small patch just changes around the code slightly to
make it easier to understand that the cases were handling diamond
shaped BB for both do_store_elim/do_hoist_loads.
There is no effect on code output at all since all of the checks
are the same still.
Note this depends on
https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616241.html
as that allows this to be done.
OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
gcc/ChangeLog:
* tree-ssa-phiopt.cc (tree_ssa_phiopt_worker):
Change the code around slightly to move diamond
handling for do_store_elim/do_hoist_loads out of
the big if/else.
---
gcc/tree-ssa-phiopt.cc | 46 +++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 25 deletions(-)
Comments
On 4/19/23 19:05, Andrew Pinski via Gcc-patches wrote:
> This small patch just changes around the code slightly to
> make it easier to understand that the cases were handling diamond
> shaped BB for both do_store_elim/do_hoist_loads.
> There is no effect on code output at all since all of the checks
> are the same still.
>
> Note this depends on
> https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616241.html
> as that allows this to be done.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (tree_ssa_phiopt_worker):
> Change the code around slightly to move diamond
> handling for do_store_elim/do_hoist_loads out of
> the big if/else.
OK.
jeff
@@ -175,28 +175,38 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
std::swap (bb1, bb2);
std::swap (e1, e2);
}
- else if (do_store_elim
- && EDGE_SUCC (bb1, 0)->dest == EDGE_SUCC (bb2, 0)->dest)
+ else if (EDGE_SUCC (bb1, 0)->dest == EDGE_SUCC (bb2, 0)->dest)
+ {
+ diamond_p = true;
+ e2 = EDGE_SUCC (bb2, 0);
+ }
+ else
+ continue;
+
+ e1 = EDGE_SUCC (bb1, 0);
+
+ /* Make sure that bb1 is just a fall through. */
+ if (!single_succ_p (bb1)
+ || (e1->flags & EDGE_FALLTHRU) == 0)
+ continue;
+
+ if (do_store_elim && diamond_p)
{
- basic_block bb3 = EDGE_SUCC (bb1, 0)->dest;
+ basic_block bb3 = e1->dest;
- if (!single_succ_p (bb1)
- || (EDGE_SUCC (bb1, 0)->flags & EDGE_FALLTHRU) == 0
- || !single_succ_p (bb2)
- || (EDGE_SUCC (bb2, 0)->flags & EDGE_FALLTHRU) == 0
+ if (!single_succ_p (bb2)
+ || (e2->flags & EDGE_FALLTHRU) == 0
|| EDGE_COUNT (bb3->preds) != 2)
continue;
if (cond_if_else_store_replacement (bb1, bb2, bb3))
cfgchanged = true;
continue;
}
- else if (do_hoist_loads
- && EDGE_SUCC (bb1, 0)->dest == EDGE_SUCC (bb2, 0)->dest)
+ else if (do_hoist_loads && diamond_p)
{
- basic_block bb3 = EDGE_SUCC (bb1, 0)->dest;
+ basic_block bb3 = e1->dest;
if (!FLOAT_TYPE_P (TREE_TYPE (gimple_cond_lhs (cond_stmt)))
- && single_succ_p (bb1)
&& single_succ_p (bb2)
&& single_pred_p (bb1)
&& single_pred_p (bb2)
@@ -209,20 +219,6 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
hoist_adjacent_loads (bb, bb1, bb2, bb3);
continue;
}
- else if (EDGE_SUCC (bb1, 0)->dest == EDGE_SUCC (bb2, 0)->dest)
- {
- diamond_p = true;
- e2 = EDGE_SUCC (bb2, 0);
- }
- else
- continue;
-
- e1 = EDGE_SUCC (bb1, 0);
-
- /* Make sure that bb1 is just a fall through. */
- if (!single_succ_p (bb1)
- || (e1->flags & EDGE_FALLTHRU) == 0)
- continue;
if (do_store_elim && !diamond_p)
{