The PR highlights several places where IVOPTs gets lost in tracking
an appropriate base to use for a TARGET_MEM_REF. The following
addresses the initial place, enough to fix the bug, but already
showing difficulties in avoiding fallout. The first place is
alloc_iv where we convert the nice pointer IV to an unsigned integer
before applying affine canonicalization. Removing that causes
gcc.target/i386/pr115462.c to regress because aff_combination_to_tree
invokes fold which eventually, with unlucky association, triggers
fold_plusminus_mult_expr and factoring resulting in missed IV
commoning. There's also some important part of fold_plusminus_mult_expr
which, when removed, regresses things again (it's all about costs).
Specifically this is factoring a * 4 - b * 4 to (a - b) * 4. After
the alloc_iv hunk this becomes a * 4 + b * -4U which isn't currently
handled, so I've hacked that in.
The alloc_iv issue was triggered by a somewhat recent change from Tamar
for GCC 15 where IIRC I suggested IVOPTs probably doesn't want to go back to
GENERIC but keep the representation as affine during analysis at least,
but that's a big change.
I'm a bit of at a loss what to do about gcc-16, we can possibly live
with some IVOPTs quality regressions by just picking the alloc_iv
change? As for fold_plusminus_mult_expr in itself I'm not sure,
suggestions welcome ...
Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu.
As said, I've run over various problematic places in affine and
IVOPTs with regard to "base object"s, but before I fix those
with mostly ignoring regressions thinking about this bug and
16 sounds like a good idea.
Thanks,
Richard.
PR tree-optimization/125730
* tree-ssa-loop-ivopts.cc (alloc_iv): Do not convert IVs to
unsigned before canonicalizing.
* fold-const.h (want_fold_plusminus_mult_expr): Declare.
* fold-const.cc (want_fold_plusminus_mult_expr): Define.
(fold_plusminus_mult_expr): Restrict parts of it. Extend
A * CST1 +- B * CST2 handling to allow for unsigned 64bit
values with sign bit set.
* tree-affine.cc (add_elt_to_tree): Unset
want_fold_plusminus_mult_expr around folding of PLUS/MINUS_EXPR.
* gcc.dg/torture/pr125730.c: New testcase.
---
gcc/fold-const.cc | 14 +++++----
gcc/fold-const.h | 2 ++
gcc/testsuite/gcc.dg/torture/pr125730.c | 40 +++++++++++++++++++++++++
gcc/tree-affine.cc | 18 +++++++++--
gcc/tree-ssa-loop-ivopts.cc | 8 ++---
5 files changed, 69 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr125730.c
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 7f49126c95a..3f94180e0d2 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -7198,6 +7198,8 @@ fold_to_nonsharp_ineq_using_bound (location_t loc, tree
ineq, tree bound)
return fold_build2_loc (loc, GE_EXPR, type, a, y);
}
+bool want_fold_plusminus_mult_expr = true;
+
/* Fold a sum or difference of at least one multiplication.
Returns the folded tree or NULL if no simplification could be made. */
@@ -7237,7 +7239,8 @@ fold_plusminus_mult_expr (location_t loc, enum tree_code
code, tree type,
arg10 = TREE_OPERAND (arg1, 0);
arg11 = TREE_OPERAND (arg1, 1);
}
- else if (TREE_CODE (arg1) == INTEGER_CST)
+ else if (TREE_CODE (arg1) == INTEGER_CST
+ && want_fold_plusminus_mult_expr)
{
arg10 = build_one_cst (type);
/* As we canonicalize A - 2 to A + -2 get rid of that sign for
@@ -7255,7 +7258,7 @@ fold_plusminus_mult_expr (location_t loc, enum tree_code
code, tree type,
else
{
/* We cannot generate constant 1 for fract. */
- if (ALL_FRACT_MODE_P (TYPE_MODE (type)))
+ if (ALL_FRACT_MODE_P (TYPE_MODE (type)) ||
!want_fold_plusminus_mult_expr)
return NULL_TREE;
arg10 = arg1;
arg11 = build_one_cst (type);
@@ -7275,10 +7278,11 @@ fold_plusminus_mult_expr (location_t loc, enum
tree_code code, tree type,
/* No identical multiplicands; see if we can find a common
power-of-two factor in non-power-of-two multiplies. This
can help in multi-dimensional array access. */
- else if (tree_fits_shwi_p (arg01) && tree_fits_shwi_p (arg11))
+ else if ((tree_fits_shwi_p (arg01) || tree_fits_uhwi_p (arg01))
+ && (tree_fits_shwi_p (arg11) || tree_fits_uhwi_p (arg11)))
{
- HOST_WIDE_INT int01 = tree_to_shwi (arg01);
- HOST_WIDE_INT int11 = tree_to_shwi (arg11);
+ HOST_WIDE_INT int01 = TREE_INT_CST_LOW (arg01);
+ HOST_WIDE_INT int11 = TREE_INT_CST_LOW (arg11);
HOST_WIDE_INT tmp;
bool swap = false;
tree maybe_same;
diff --git a/gcc/fold-const.h b/gcc/fold-const.h
index 57d32b1b6ca..ddf7718d1d7 100644
--- a/gcc/fold-const.h
+++ b/gcc/fold-const.h
@@ -31,6 +31,8 @@ extern int folding_initializer;
during folding in that context. */
extern bool folding_cxx_constexpr;
+extern bool want_fold_plusminus_mult_expr;
+
/* Convert between trees and native memory representation. */
extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
extern int native_encode_initializer (tree, unsigned char *, int,
diff --git a/gcc/testsuite/gcc.dg/torture/pr125730.c
b/gcc/testsuite/gcc.dg/torture/pr125730.c
new file mode 100644
index 00000000000..ef698e4f513
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr125730.c
@@ -0,0 +1,40 @@
+/* { dg-do run } */
+
+char *key;
+char *value;
+char **name_regex = &key;
+
+__attribute__ ((noipa)) void
+dict_get_item_ref (char ***result)
+{
+ *result = &value;
+}
+
+__attribute__ ((noinline)) static void
+parse_keyword_dict (char ***argnames, char ***values,
+ int num_posargs, int num_kwargs)
+{
+ char ***first_kw_arg = argnames + num_posargs;
+ char ***name = first_kw_arg;
+
+ while (*name && num_kwargs > 0)
+ {
+ char **fetched_value;
+ dict_get_item_ref (&fetched_value);
+ values[name - argnames] = fetched_value;
+ name++;
+ }
+}
+
+int
+main ()
+{
+ char **fetched_values[] = { 0 };
+ char **argnames[] = { name_regex, 0 };
+ parse_keyword_dict (argnames, fetched_values, 0, 1);
+
+ if (fetched_values[0] != &value)
+ __builtin_abort ();
+
+ return 0;
+}
diff --git a/gcc/tree-affine.cc b/gcc/tree-affine.cc
index b5b7249c675..531625a0520 100644
--- a/gcc/tree-affine.cc
+++ b/gcc/tree-affine.cc
@@ -494,7 +494,11 @@ add_elt_to_tree (tree expr, tree type, tree elt, const
widest_int &scale_in)
if (!expr)
return elt;
- return fold_build2 (PLUS_EXPR, type, expr, elt);
+ bool saved_want_fold_plusminus_mult_expr = want_fold_plusminus_mult_expr;
+ want_fold_plusminus_mult_expr = false;
+ elt = fold_build2 (PLUS_EXPR, type, expr, elt);
+ want_fold_plusminus_mult_expr = saved_want_fold_plusminus_mult_expr;
+ return elt;
}
if (scale == -1)
@@ -502,7 +506,11 @@ add_elt_to_tree (tree expr, tree type, tree elt, const
widest_int &scale_in)
if (!expr)
return fold_build1 (NEGATE_EXPR, type, elt);
- return fold_build2 (MINUS_EXPR, type, expr, elt);
+ bool saved_want_fold_plusminus_mult_expr = want_fold_plusminus_mult_expr;
+ want_fold_plusminus_mult_expr = false;
+ elt = fold_build2 (MINUS_EXPR, type, expr, elt);
+ want_fold_plusminus_mult_expr = saved_want_fold_plusminus_mult_expr;
+ return elt;
}
if (!expr)
@@ -517,7 +525,11 @@ add_elt_to_tree (tree expr, tree type, tree elt, const
widest_int &scale_in)
code = PLUS_EXPR;
elt = fold_build2 (MULT_EXPR, type, elt, wide_int_to_tree (type, scale));
- return fold_build2 (code, type, expr, elt);
+ bool saved_want_fold_plusminus_mult_expr = want_fold_plusminus_mult_expr;
+ want_fold_plusminus_mult_expr = false;
+ elt = fold_build2 (code, type, expr, elt);
+ want_fold_plusminus_mult_expr = saved_want_fold_plusminus_mult_expr;
+ return elt;
}
/* Makes tree from the affine combination COMB. */
diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
index 5c209ab77dc..15221b55e6f 100644
--- a/gcc/tree-ssa-loop-ivopts.cc
+++ b/gcc/tree-ssa-loop-ivopts.cc
@@ -1162,8 +1162,8 @@ alloc_iv (struct ivopts_data *data, tree base, tree step,
sizeof (struct iv));
gcc_assert (step != NULL_TREE);
- /* Canonicalize the address expression in base if it were an unsigned
- computation. That leads to more equalities being detected and results in:
+ /* Canonicalize the address expression in base.
+ That leads to more equalities being detected and results in:
1) More accurate cost can be computed for address expressions;
2) Duplicate candidates won't be created for bases in different
@@ -1171,10 +1171,8 @@ alloc_iv (struct ivopts_data *data, tree base, tree step,
3) Duplicate candidates won't be created for IV expressions that differ
only in their sign. */
aff_tree comb;
- STRIP_NOPS (expr);
- expr = fold_convert (unsigned_type_for (TREE_TYPE (expr)), expr);
tree_to_aff_combination (expr, TREE_TYPE (expr), &comb);
- base = fold_convert (TREE_TYPE (base), aff_combination_to_tree (&comb));
+ base = aff_combination_to_tree (&comb);
iv->base = base;
iv->base_object = determine_base_object (data, base);
--
2.51.0
