From: Kyrylo Tkachov <[email protected]>
factor_out_conditional_load currently only runs from the sink pass. Also call
it from phiopt itself when the join block of a diamond load PHI <*P, *Q> lies
outside any loop: there is nothing to loop-vectorise there, so commoning the
two arm loads into a single P' = PHI <P, Q>; result = *P' is a pure win and is
cheap to attempt.
This is restricted to:
* the late phiopt run (!early_p), since after inlining the region may yet
become part of a loop where the commoned load could inhibit vectorisation;
* SSA-name arm pointers (a new REQUIRE_SSA_ARGS argument to
factor_out_conditional_load), so that phiprop does not undo it.
This triggers a few 100s times in SPEC2026 with no harmful effects. The
llvm benchmarks improves slightly on my aarch64 machine (1.6%)
Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk after the prerequisites?
Thanks,
Kyrill
Signed-off-by: Kyrylo Tkachov <[email protected]>
gcc/
PR tree-optimization/125557
* tree-ssa-phiopt.h (conditional_load_factorable_p)
(factor_out_conditional_load): Add a REQUIRE_SSA_ARGS parameter
defaulting to false.
* tree-ssa-phiopt.cc (conditional_load_factorable_p): Honour
REQUIRE_SSA_ARGS.
(factor_out_conditional_load): Pass it through.
(pass_phiopt::execute): Call factor_out_conditional_load for
out-of-loop diamonds in the late run.
gcc/testsuite/
PR tree-optimization/125557
* gcc.dg/tree-ssa/phiopt-factor-load-1.c: New test.
---
.../gcc.dg/tree-ssa/phiopt-factor-load-1.c | 22 ++++++++++
gcc/tree-ssa-phiopt.cc | 42 ++++++++++++++-----
gcc/tree-ssa-phiopt.h | 8 ++--
3 files changed, 59 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c
b/gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c
new file mode 100644
index 00000000000..fe6a5e9cf3f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-phiopt2" } */
+
+/* PR tree-optimization/125557: outside any loop a diamond load PHI <*p, *q> is
+ factored by phiopt itself (not only by the sink pass) into a pointer PHI
+ P' = PHI <p, q> followed by a single load *P', removing the duplicate load.
+ The arm pointers here are SSA names, so phiprop does not undo it. */
+
+int
+f (int *p, int *q, int c)
+{
+ int x;
+ if (c)
+ x = *p;
+ else
+ x = *q;
+ return x;
+}
+
+/* The two arm loads collapse to a pointer PHI feeding one load of its result.
*/
+/* { dg-final { scan-tree-dump {= PHI <p_[0-9]+\(D\)} "phiopt2" } } */
+/* { dg-final { scan-tree-dump-times {= \*_[0-9]+;} 1 "phiopt2" } } */
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index c2cdc54f977..44523267626 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -437,13 +437,15 @@ remove_factored_arm_defs (gimple *d0, gimple *d1)
}
/* Return true if factor_out_conditional_load (below) would factor the load PHI
- at MERGE. E0/E1 are the arm->MERGE edges. This is
factor_out_conditional_load's
- eligibility test, split out so callers (the sink pass) can gate on
commonability
- without mutating: the two MUST stay in lockstep, otherwise a caller could
common
- a load the sink then cannot finish if-converting. */
+ at MERGE. E0/E1 are the arm->MERGE edges; when REQUIRE_SSA_ARGS the two arm
+ pointers must themselves be SSA names. This is
factor_out_conditional_load's
+ eligibility test, split out so callers (the sink pass) can gate on
+ commonability without mutating: the two MUST stay in lockstep, otherwise a
+ caller could common a load the sink then cannot finish if-converting. */
bool
-conditional_load_factorable_p (edge e0, edge e1, basic_block merge, gphi *phi)
+conditional_load_factorable_p (edge e0, edge e1, basic_block merge, gphi *phi,
+ bool require_ssa_args)
{
if (virtual_operand_p (gimple_phi_result (phi))
|| gimple_phi_num_args (phi) != 2
@@ -480,6 +482,13 @@ conditional_load_factorable_p (edge e0, edge e1,
basic_block merge, gphi *phi)
if (!types_compatible_p (TREE_TYPE (p0), TREE_TYPE (p1)))
return false;
+ /* The phiopt caller (REQUIRE_SSA_ARGS) only commons SSA-name pointers: with
a
+ non-SSA arm pointer phiprop would propagate the load back into the arms
and
+ undo the transform. The sink caller has no such issue and passes false.
*/
+ if (require_ssa_args
+ && (TREE_CODE (p0) != SSA_NAME || TREE_CODE (p1) != SSA_NAME))
+ return false;
+
return true;
}
@@ -489,12 +498,15 @@ conditional_load_factorable_p (edge e0, edge e1,
basic_block merge, gphi *phi)
load *P' replacing the PHI. No speculative load is introduced (the load
uses
whichever pointer the taken edge selected). Together with operand factoring
this turns a conditional load into one reg-offset load. E0/E1 are the
- arm->MERGE edges. Returns true if the load was factored out. */
+ arm->MERGE edges. REQUIRE_SSA_ARGS is as in conditional_load_factorable_p
+ (the phiopt caller sets it so phiprop cannot undo the transform). Returns
+ true if the load was factored out. */
bool
-factor_out_conditional_load (edge e0, edge e1, basic_block merge, gphi *phi)
+factor_out_conditional_load (edge e0, edge e1, basic_block merge, gphi *phi,
+ bool require_ssa_args)
{
- if (!conditional_load_factorable_p (e0, e1, merge, phi))
+ if (!conditional_load_factorable_p (e0, e1, merge, phi, require_ssa_args))
return false;
/* Re-derive the loads and pointers validated by the predicate above. */
@@ -4407,8 +4419,18 @@ pass_phiopt::execute (function *)
{
gphi *phi = as_a <gphi *> (gsi_stmt (gsi));
- if (factor_out_conditional_operation (e1, e2, merge, phi,
- cond_stmt, early_p))
+ bool factored
+ = factor_out_conditional_operation (e1, e2, merge, phi,
+ cond_stmt, early_p);
+ /* Outside any loop a diamond load PHI <*P, *Q> cannot be
+ loop-vectorised, so commoning it into P' = PHI <P, Q>; *P' is a
+ pure win. Do it only when not in the early run (the region may
+ yet become a loop after inlining) and only for SSA-name
pointers
+ so phiprop cannot undo it. */
+ if (!factored && !early_p && bb_loop_depth (merge) == 0)
+ factored = factor_out_conditional_load (e1, e2, merge, phi,
+
/*require_ssa_args=*/true);
+ if (factored)
{
/* Start over if there was an operation that was factored out
because the new phi might have another opportunity. */
phis = phi_nodes (merge);
diff --git a/gcc/tree-ssa-phiopt.h b/gcc/tree-ssa-phiopt.h
index 7383095f5a5..8f9b9edcdad 100644
--- a/gcc/tree-ssa-phiopt.h
+++ b/gcc/tree-ssa-phiopt.h
@@ -25,11 +25,13 @@ extern int find_different_opnum (const gimple_match_op
&arg0_op,
/* True if factor_out_conditional_load would factor the load PHI at MERGE; lets
the sink pass gate on commonability without mutating. */
extern bool conditional_load_factorable_p (edge e0, edge e1, basic_block merge,
- gphi *phi);
+ gphi *phi,
+ bool require_ssa_args = false);
/* Factor a load out of a "load PHI" PHI <*P, *Q> into P' = PHI <P, Q>;
- result = *P'. */
+ result = *P'. When REQUIRE_SSA_ARGS, only do so when P and Q are SSA names
+ (the phiopt caller sets this so phiprop cannot undo the transform). */
extern bool factor_out_conditional_load (edge e0, edge e1, basic_block merge,
- gphi *phi);
+ gphi *phi, bool require_ssa_args =
false);
/* Factor a common operation (unary, conversion, or N-ary with one differing
operand) out of a 2-argument PHI. */
extern bool factor_out_conditional_operation (edge e0, edge e1,
--
2.50.1 (Apple Git-155)
