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 (factor_out_conditional_load): Add a
REQUIRE_SSA_ARGS parameter defaulting to false.
* tree-ssa-phiopt.cc (factor_out_conditional_load): Honour
REQUIRE_SSA_ARGS.
(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 | 28 ++++++++++++++++---
gcc/tree-ssa-phiopt.h | 5 ++--
3 files changed, 49 insertions(+), 6 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 f467a9d54fe..35033cd6d4f 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -442,10 +442,13 @@ remove_factored_arm_defs (gimple *d0, gimple *d1)
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. When REQUIRE_SSA_ARGS the two arm pointers must
themselves
+ be SSA names (the phiopt caller sets this 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 (virtual_operand_p (gimple_phi_result (phi))
|| gimple_phi_num_args (phi) != 2
@@ -482,6 +485,13 @@ factor_out_conditional_load (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;
+
/* Build P' = PHI <P, Q> and the single load result = *P'. */
tree res = gimple_phi_result (phi);
tree ptr = make_ssa_name (TREE_TYPE (p0));
@@ -4384,8 +4394,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 8b510b34aa6..b46a8858b14 100644
--- a/gcc/tree-ssa-phiopt.h
+++ b/gcc/tree-ssa-phiopt.h
@@ -23,9 +23,10 @@ extern int find_different_opnum (const gimple_match_op
&arg0_op,
tree *new_arg0, tree *new_arg1);
/* 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)
