From 845ee9c7107956845e487cb123fa581d9c70ea1b Mon Sep 17 00:00:00 2001 From: Richard Biener Date: Fri, 25 Aug 2023 13:37:30 +0200 Subject: [PATCH] tree-optimization/111137 - dependence checking for SLP The following fixes a mistake with SLP dependence checking. When checking whether we can hoist loads to the first load place we special-case stores of the same instance considering them sunk to the last store place. But we fail to consider that stores from other SLP instances are sunk in a similar way. This leads us to miss the dependence between (A) and (B) in b[0][1] = 0; (A) ... _6 = b[_5 /* 0 */][0]; (B') _7 = _6 ^ 1; b[_5 /* 0 */][0] = _7; b[0][2] = 0; (A') _10 = b[_5 /* 0 */][1]; (B) _11 = _10 ^ 1; b[_5 /* 0 */][1] = _11; where the zeroing stores are sunk to (A') and the loads hoisted to (B'). The following fixes this, treating grouped stores from other instances similar to stores from our own instance. The difference is - and this is more conservative than necessary - that we don't know which stores of a group are in which SLP instance (though I believe either all of the grouped stores will be in a single SLP instance or in none at the moment), so we don't know which stores are sunk where. We simply assume they are all sunk to the last store we run into. Likewise we do not take into account that an SLP instance might be cancelled (or a grouped store not actually belong to any instance). PR tree-optimization/111137 * tree-vect-data-refs.cc (vect_slp_analyze_load_dependences): Properly handle grouped stores from other SLP instances. * gcc.dg/torture/pr111137.c: New testcase. --- gcc/testsuite/gcc.dg/torture/pr111137.c | 30 ++++++++++++++++ gcc/tree-vect-data-refs.cc | 64 ++++++++++++++++++++++++--------- 2 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr111137.c diff --git a/gcc/testsuite/gcc.dg/torture/pr111137.c b/gcc/testsuite/gcc.dg/torture/pr111137.c new file mode 100644 index 00000000000..77560487926 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr111137.c @@ -0,0 +1,30 @@ +/* { dg-do run } */ + +int b[3][8]; +short d; +volatile int t = 1; + +void __attribute__((noipa)) +foo() +{ + int g = t; + for (int e = 1; e >= 0; e--) + { + d = 1; + for (; d >= 0; d--) + { + b[0][d * 2 + 1] = 0; + b[g - 1 + d][0] ^= 1; + b[0][d * 2 + 2] = 0; + b[g - 1 + d][1] ^= 1; + } + } +} + +int +main() +{ + foo (); + if (b[0][1] != 1) + __builtin_abort(); +} diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc index 0295e256a44..40ab568fe35 100644 --- a/gcc/tree-vect-data-refs.cc +++ b/gcc/tree-vect-data-refs.cc @@ -750,6 +750,7 @@ vect_slp_analyze_load_dependences (vec_info *vinfo, slp_tree node, data_reference *dr_a = STMT_VINFO_DATA_REF (access_info); ao_ref ref; bool ref_initialized_p = false; + hash_set grp_visited; for (gimple_stmt_iterator gsi = gsi_for_stmt (access_info->stmt); gsi_stmt (gsi) != first_access_info->stmt; gsi_prev (&gsi)) { @@ -782,26 +783,55 @@ vect_slp_analyze_load_dependences (vec_info *vinfo, slp_tree node, continue; } - /* We are hoisting a load - this means we can use TBAA for - disambiguation. */ - if (!ref_initialized_p) - ao_ref_init (&ref, DR_REF (dr_a)); - if (stmt_may_clobber_ref_p_1 (stmt, &ref, true)) + auto check_hoist = [&] (stmt_vec_info stmt_info) -> bool { - /* If we couldn't record a (single) data reference for this - stmt we have to give up now. */ - data_reference *dr_b = STMT_VINFO_DATA_REF (stmt_info); - if (!dr_b) - return false; - ddr_p ddr = initialize_data_dependence_relation (dr_a, - dr_b, vNULL); - bool dependent - = vect_slp_analyze_data_ref_dependence (vinfo, ddr); - free_dependence_relation (ddr); - if (dependent) + /* We are hoisting a load - this means we can use TBAA for + disambiguation. */ + if (!ref_initialized_p) + ao_ref_init (&ref, DR_REF (dr_a)); + if (stmt_may_clobber_ref_p_1 (stmt_info->stmt, &ref, true)) + { + /* If we couldn't record a (single) data reference for this + stmt we have to give up now. */ + data_reference *dr_b = STMT_VINFO_DATA_REF (stmt_info); + if (!dr_b) + return false; + ddr_p ddr = initialize_data_dependence_relation (dr_a, + dr_b, vNULL); + bool dependent + = vect_slp_analyze_data_ref_dependence (vinfo, ddr); + free_dependence_relation (ddr); + if (dependent) + return false; + } + /* No dependence. */ + return true; + }; + if (STMT_VINFO_GROUPED_ACCESS (stmt_info)) + { + /* When we run into a store group we have to honor + that earlier stores might be moved here. We don't + know exactly which and where to since we lack a + back-mapping from DR to SLP node, so assume all + earlier stores are sunk here. It's enough to + consider the last stmt of a group for this. + ??? Both this and the fact that we disregard that + the conflicting instance might be removed later + is overly conservative. */ + if (!grp_visited.add (DR_GROUP_FIRST_ELEMENT (stmt_info))) + for (auto store_info = DR_GROUP_FIRST_ELEMENT (stmt_info); + store_info != NULL; + store_info = DR_GROUP_NEXT_ELEMENT (store_info)) + if ((store_info == stmt_info + || get_later_stmt (store_info, stmt_info) == stmt_info) + && !check_hoist (store_info)) + return false; + } + else + { + if (!check_hoist (stmt_info)) return false; } - /* No dependence. */ } } return true; -- 2.11.4.GIT