https://gcc.gnu.org/g:19dd791b3a7166df0766dfd0b5e6918f8e3d1bba
commit r14-11682-g19dd791b3a7166df0766dfd0b5e6918f8e3d1bba
Author: Martin Jambor <mjam...@suse.cz>
Date: Mon Apr 7 13:32:09 2025 +0200
sra: Avoid creating TBAA hazards (PR118924)
The testcase in PR 118924, when compiled on Aarch64, contains an
gimple aggregate assignment statement in between different types which
are types_compatible_p but behave differently for the purposes of
alias analysis.
SRA replaces the statement with a series of scalar assignments which
however have LHSs access chains modeled on the RHS type and so do not
alias with a subsequent reads and so are DSEd.
SRA clearly gets its "same_access_path" logic subtly wrong. One issue
is that the same_access_path_p function probably should be implemented
more along the lines of (parts of ao_compare::compare_ao_refs) instead
of internally relying on operand_equal_p. That is however not the
problem in the PR and so I will deal with it only later.
The issue here is that even when the access path is the same, it must
not be bolted on an aggregate type that does not match. This patch
does that, taking just one simple function from the
ao_compare::compare_ao_refs machinery and using it to detect the
situation. The rest is just merging the information in between
accesses of the same access group.
I looked at how many times we come across such assignment during
"make stage2-bubble" of GCC (configured with only c and C++ and
without multilib and libsanitizers) and on an x86_64 there were 87924
such assignments (though now I realize not all of them had to be
aggregate), so they do happen. The patch leads to about 5% increase
of cases where we don't use an "access path" but resort to a
MEM_REF (from 90209 to 95204). On an Aarch64, there were 92268 such
assignments and the increase of falling back to MEM_REFs was by
4% (but from a bigger base 132983 to 107991).
gcc/ChangeLog:
2025-04-04 Martin Jambor <mjam...@suse.cz>
PR tree-optimization/118924
* tree-ssa-alias-compare.h (types_equal_for_same_type_for_tbaa_p):
Declare.
* tree-ssa-alias.cc: Include ipa-utils.h.
(types_equal_for_same_type_for_tbaa_p): New public overloaded
variant.
* tree-sra.cc: Include tree-ssa-alias-compare.h.
(create_access): Initialzie grp_same_access_path to true.
(build_accesses_from_assign): Detect tbaa hazards and clear
grp_same_access_path fields of involved accesses when they occur.
(sort_and_splice_var_accesses): Take previous values of
grp_same_access_path into account.
gcc/testsuite/ChangeLog:
2025-03-25 Martin Jambor <mjam...@suse.cz>
PR tree-optimization/118924
* g++.dg/tree-ssa/pr118924.C: New test.
(cherry picked from commit 07d243670020b339380194f6125cde87ada56148)
Diff:
---
gcc/testsuite/g++.dg/tree-ssa/pr118924.C | 29 +++++++++++++++++++++++++++++
gcc/tree-sra.cc | 17 ++++++++++++++---
gcc/tree-ssa-alias-compare.h | 2 ++
gcc/tree-ssa-alias.cc | 13 ++++++++++++-
4 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr118924.C
b/gcc/testsuite/g++.dg/tree-ssa/pr118924.C
new file mode 100644
index 000000000000..c95eacafc9ce
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr118924.C
@@ -0,0 +1,29 @@
+/* { dg-do run } */
+/* { dg-options "-std=c++17 -O2" } */
+
+template <int Size> struct Vector {
+ int m_data[Size];
+ Vector(int, int, int) {}
+};
+enum class E { POINTS, LINES, TRIANGLES };
+
+__attribute__((noipa))
+void getName(E type) {
+ static E check = E::POINTS;
+ if (type == check)
+ check = (E)((int)check + 1);
+ else
+ __builtin_abort ();
+}
+
+int main() {
+ int arr[]{0, 1, 2};
+ for (auto dim : arr) {
+ Vector<3> localInvs(1, 1, 1);
+ localInvs.m_data[dim] = 8;
+ }
+ E types[] = {E::POINTS, E::LINES, E::TRIANGLES};
+ for (auto primType : types)
+ getName(primType);
+ return 0;
+}
diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index c91e40ef7e71..e1243dd0441d 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -100,6 +100,7 @@ along with GCC; see the file COPYING3. If not see
#include "builtins.h"
#include "tree-sra.h"
#include "opts.h"
+#include "tree-ssa-alias-compare.h"
/* Enumeration of all aggregate reductions we can do. */
enum sra_mode { SRA_MODE_EARLY_IPA, /* early call regularization */
@@ -979,6 +980,7 @@ create_access (tree expr, gimple *stmt, bool write)
access->type = TREE_TYPE (expr);
access->write = write;
access->grp_unscalarizable_region = unscalarizable_region;
+ access->grp_same_access_path = true;
access->stmt = stmt;
access->reverse = reverse;
@@ -1522,6 +1524,9 @@ build_accesses_from_assign (gimple *stmt)
racc = build_access_from_expr_1 (rhs, stmt, false);
lacc = build_access_from_expr_1 (lhs, stmt, true);
+ bool tbaa_hazard
+ = !types_equal_for_same_type_for_tbaa_p (TREE_TYPE (lhs), TREE_TYPE (rhs));
+
if (lacc)
{
lacc->grp_assignment_write = 1;
@@ -1536,6 +1541,8 @@ build_accesses_from_assign (gimple *stmt)
bitmap_set_bit (cannot_scalarize_away_bitmap,
DECL_UID (lacc->base));
}
+ if (tbaa_hazard)
+ lacc->grp_same_access_path = false;
}
if (racc)
@@ -1555,6 +1562,8 @@ build_accesses_from_assign (gimple *stmt)
}
if (storage_order_barrier_p (lhs))
racc->grp_unscalarizable_region = 1;
+ if (tbaa_hazard)
+ racc->grp_same_access_path = false;
}
if (lacc && racc
@@ -2383,7 +2392,7 @@ sort_and_splice_var_accesses (tree var)
bool grp_partial_lhs = access->grp_partial_lhs;
bool first_scalar = is_gimple_reg_type (access->type);
bool unscalarizable_region = access->grp_unscalarizable_region;
- bool grp_same_access_path = true;
+ bool grp_same_access_path = access->grp_same_access_path;
bool bf_non_full_precision
= (INTEGRAL_TYPE_P (access->type)
&& TYPE_PRECISION (access->type) != access->size
@@ -2419,7 +2428,8 @@ sort_and_splice_var_accesses (tree var)
return NULL;
}
- grp_same_access_path = path_comparable_for_same_access (access->expr);
+ if (grp_same_access_path)
+ grp_same_access_path = path_comparable_for_same_access (access->expr);
j = i + 1;
while (j < access_count)
@@ -2472,7 +2482,8 @@ sort_and_splice_var_accesses (tree var)
}
if (grp_same_access_path
- && !same_access_path_p (access->expr, ac2->expr))
+ && (!ac2->grp_same_access_path
+ || !same_access_path_p (access->expr, ac2->expr)))
grp_same_access_path = false;
ac2->group_representative = access;
diff --git a/gcc/tree-ssa-alias-compare.h b/gcc/tree-ssa-alias-compare.h
index 529644bccb89..b7f8f35f395c 100644
--- a/gcc/tree-ssa-alias-compare.h
+++ b/gcc/tree-ssa-alias-compare.h
@@ -40,4 +40,6 @@ class ao_compare : public operand_compare
inchash::hash &hstate);
};
+bool types_equal_for_same_type_for_tbaa_p (tree type1, tree type2);
+
#endif
diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
index beab6249ae43..790447b38403 100644
--- a/gcc/tree-ssa-alias.cc
+++ b/gcc/tree-ssa-alias.cc
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-ssa-alias-compare.h"
#include "builtins.h"
#include "internal-fn.h"
+#include "ipa-utils.h"
/* Broad overview of how alias analysis on gimple works:
@@ -4112,7 +4113,7 @@ attr_fnspec::verify ()
}
/* Return ture if TYPE1 and TYPE2 will always give the same answer
- when compared wit hother types using same_type_for_tbaa_p. */
+ when compared with other types using same_type_for_tbaa. */
static bool
types_equal_for_same_type_for_tbaa_p (tree type1, tree type2,
@@ -4135,6 +4136,16 @@ types_equal_for_same_type_for_tbaa_p (tree type1, tree
type2,
return TYPE_CANONICAL (type1) == TYPE_CANONICAL (type2);
}
+/* Return ture if TYPE1 and TYPE2 will always give the same answer
+ when compared with other types using same_type_for_tbaa. */
+
+bool
+types_equal_for_same_type_for_tbaa_p (tree type1, tree type2)
+{
+ return types_equal_for_same_type_for_tbaa_p (type1, type2,
+ lto_streaming_expected_p ());
+}
+
/* Compare REF1 and REF2 and return flags specifying their differences.
If LTO_STREAMING_SAFE is true do not use alias sets and canonical
types that are going to be recomputed.
