From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 9E4C83858403 for ; Sun, 14 Nov 2021 11:05:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9E4C83858403 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 82CDF280941; Sun, 14 Nov 2021 12:05:28 +0100 (CET) Date: Sun, 14 Nov 2021 12:05:28 +0100 From: Jan Hubicka To: gcc-patches@gcc.gnu.org Subject: Re: Cleanup hadnling of modref access_nodes in tree-ssa-alias and tree-ssa-dse Message-ID: <20211114110528.GN13726@kam.mff.cuni.cz> References: <20211114102912.GK13726@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211114102912.GK13726@kam.mff.cuni.cz> User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Nov 2021 11:05:31 -0000 Hi, this is variant I comitted. Commonizing the code exposed that I can drop memory walking when parameter passed is NULL (under assumption of flag_delete_null_pointer_checks) since it can not point to useful memory. This was already done in tree-ssa-alias, but not in tree-ssa-dse. This needed bit of testsuite compensation for cases where we optimize out invalid memory accesses. Bootstrapped/regtested x86_64-linux, comitted. gcc/ChangeLog: 2021-11-14 Jan Hubicka * ipa-modref-tree.c (modref_access_node::get_call_arg): New member function. (modref_access_node::get_ao_ref): Likewise. * ipa-modref-tree.h (modref_access_node::get_call_arg): Declare. (modref_access_node::get_ao_ref): Declare. * tree-ssa-alias.c (modref_may_conflict): Use new accessors. * tree-ssa-dse.c (dse_optimize_call): Use new accessors. gcc/testsuite/ChangeLog: 2021-11-14 Jan Hubicka * c-c++-common/asan/null-deref-1.c: Update template. * c-c++-common/tsan/free_race.c: Update template. * c-c++-common/tsan/free_race2.c: Update template. * gcc.dg/ipa/ipa-sra-4.c: Update template. diff --git a/gcc/ipa-modref-tree.c b/gcc/ipa-modref-tree.c index 70ec71c3808..6fc2b7298f4 100644 --- a/gcc/ipa-modref-tree.c +++ b/gcc/ipa-modref-tree.c @@ -25,6 +25,8 @@ along with GCC; see the file COPYING3. If not see #include "tree.h" #include "ipa-modref-tree.h" #include "selftest.h" +#include "tree-ssa-alias.h" +#include "gimple.h" /* Return true if both accesses are the same. */ bool @@ -603,6 +605,39 @@ modref_access_node::dump (FILE *out) fprintf (out, "\n"); } +/* Return tree corresponding to parameter of the range in STMT. */ +tree +modref_access_node::get_call_arg (const gcall *stmt) const +{ + if (parm_index == MODREF_UNKNOWN_PARM) + return NULL; + if (parm_index == MODREF_STATIC_CHAIN_PARM) + return gimple_call_chain (stmt); + /* MODREF_RETSLOT_PARM should not happen in access trees since the store + is seen explicitly in the caller. */ + gcc_checking_assert (parm_index >= 0); + if (parm_index >= (int)gimple_call_num_args (stmt)) + return NULL; + return gimple_call_arg (stmt, parm_index); +} + +/* Return tree corresponding to parameter of the range in STMT. */ +bool +modref_access_node::get_ao_ref (const gcall *stmt, ao_ref *ref) const +{ + tree arg; + + if (!parm_offset_known || !(arg = get_call_arg (stmt))) + return false; + poly_offset_int off = (poly_offset_int)offset + + ((poly_offset_int)parm_offset << LOG2_BITS_PER_UNIT); + poly_int64 off2; + if (!off.to_shwi (&off2)) + return false; + ao_ref_init_from_ptr_and_range (ref, arg, true, off2, size, max_size); + return true; +} + #if CHECKING_P namespace selftest { diff --git a/gcc/ipa-modref-tree.h b/gcc/ipa-modref-tree.h index 1fafd59debe..2fcabe480bd 100644 --- a/gcc/ipa-modref-tree.h +++ b/gcc/ipa-modref-tree.h @@ -77,7 +77,7 @@ struct GTY(()) modref_access_node This has to be limited in order to keep dataflow finite. */ unsigned char adjustments; - /* Return true if access node holds no useful info. */ + /* Return true if access node holds some useful info. */ bool useful_p () const { return parm_index != MODREF_UNKNOWN_PARM; @@ -88,10 +88,13 @@ struct GTY(()) modref_access_node bool operator == (modref_access_node &a) const; /* Return true if range info is useful. */ bool range_info_useful_p () const; + /* Return tree corresponding to parameter of the range in STMT. */ + tree get_call_arg (const gcall *stmt) const; + /* Build ao_ref corresponding to the access and return true if succesful. */ + bool get_ao_ref (const gcall *stmt, class ao_ref *ref) const; /* Insert A into vector ACCESSES. Limit size of vector to MAX_ACCESSES and if RECORD_ADJUSTMENT is true keep track of adjustment counts. - Return 0 if nothing changed, 1 is insertion suceeded and -1 if - failed. */ + Return 0 if nothing changed, 1 is insertion suceeded and -1 if failed. */ static int insert (vec *&accesses, modref_access_node a, size_t max_accesses, bool record_adjustments); diff --git a/gcc/testsuite/c-c++-common/asan/null-deref-1.c b/gcc/testsuite/c-c++-common/asan/null-deref-1.c index bae016d6419..c967b29b9e2 100644 --- a/gcc/testsuite/c-c++-common/asan/null-deref-1.c +++ b/gcc/testsuite/c-c++-common/asan/null-deref-1.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-options "-fno-omit-frame-pointer -fno-shrink-wrap" } */ +/* { dg-options "-fno-omit-frame-pointer -fno-shrink-wrap -fno-ipa-modref" } */ /* { dg-additional-options "-mno-omit-leaf-frame-pointer" { target { i?86-*-* x86_64-*-* } } } */ /* { dg-shouldfail "asan" } */ diff --git a/gcc/testsuite/c-c++-common/tsan/free_race.c b/gcc/testsuite/c-c++-common/tsan/free_race.c index 258f7b7420d..831c23e8859 100644 --- a/gcc/testsuite/c-c++-common/tsan/free_race.c +++ b/gcc/testsuite/c-c++-common/tsan/free_race.c @@ -1,4 +1,5 @@ /* { dg-shouldfail "tsan" } */ +/* { dg-additional-options "-ldl -fno-ipa-modref" } */ #include diff --git a/gcc/testsuite/c-c++-common/tsan/free_race2.c b/gcc/testsuite/c-c++-common/tsan/free_race2.c index 3971180c22b..a74d9dc3940 100644 --- a/gcc/testsuite/c-c++-common/tsan/free_race2.c +++ b/gcc/testsuite/c-c++-common/tsan/free_race2.c @@ -1,4 +1,5 @@ /* { dg-shouldfail "tsan" } */ +/* { dg-additional-options "-ldl -fno-ipa-modref" } */ #include diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c index fdbd5e5d72d..c86ae8320a7 100644 --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fipa-sra -fno-ipa-pure-const -fdump-ipa-sra" } */ +/* { dg-options "-O2 -fipa-sra -fno-ipa-pure-const -fdump-ipa-sra -fno-ipa-modref" } */ static int __attribute__((noinline)) diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c index 2965902912f..29be1f848b5 100644 --- a/gcc/tree-ssa-alias.c +++ b/gcc/tree-ssa-alias.c @@ -2535,13 +2535,10 @@ refs_output_dependent_p (tree store1, tree store2) IF TBAA_P is true, use TBAA oracle. */ static bool -modref_may_conflict (const gimple *stmt, +modref_may_conflict (const gcall *stmt, modref_tree *tt, ao_ref *ref, bool tbaa_p) { alias_set_type base_set, ref_set; - modref_base_node *base_node; - modref_ref_node *ref_node; - size_t i, j, k; if (tt->every_base) return true; @@ -2554,7 +2551,7 @@ modref_may_conflict (const gimple *stmt, ref_set = ao_ref_alias_set (ref); int num_tests = 0, max_tests = param_modref_max_tests; - FOR_EACH_VEC_SAFE_ELT (tt->bases, i, base_node) + for (auto base_node : tt->bases) { if (tbaa_p && flag_strict_aliasing) { @@ -2569,7 +2566,7 @@ modref_may_conflict (const gimple *stmt, if (base_node->every_ref) return true; - FOR_EACH_VEC_SAFE_ELT (base_node->refs, j, ref_node) + for (auto ref_node : base_node->refs) { /* Do not repeat same test as before. */ if ((ref_set != base_set || base_node->base != ref_node->ref) @@ -2583,66 +2580,43 @@ modref_may_conflict (const gimple *stmt, num_tests++; } - /* TBAA checks did not disambiguate, try to use base pointer, for - that we however need to have ref->ref or ref->base. */ - if (ref_node->every_access || (!ref->ref && !ref->base)) + if (ref_node->every_access) return true; - modref_access_node *access_node; - FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node) + /* TBAA checks did not disambiguate, try individual accesses. */ + for (auto access_node : ref_node->accesses) { if (num_tests >= max_tests) return true; - if (access_node->parm_index == MODREF_UNKNOWN_PARM - || access_node->parm_index - >= (int)gimple_call_num_args (stmt)) + tree arg = access_node.get_call_arg (stmt); + if (!arg) return true; alias_stats.modref_baseptr_tests++; - tree arg; - - if (access_node->parm_index == MODREF_STATIC_CHAIN_PARM) - arg = gimple_call_chain (stmt); - else - arg = gimple_call_arg (stmt, access_node->parm_index); if (integer_zerop (arg) && flag_delete_null_pointer_checks) continue; + /* PTA oracle will be unhapy of arg is not an pointer. */ if (!POINTER_TYPE_P (TREE_TYPE (arg))) return true; - /* ao_ref_init_from_ptr_and_range assumes that memory access - starts by the pointed to location. If we did not track the - offset it is possible that it starts before the actual - pointer. */ - if (!access_node->parm_offset_known) - { - if (ptr_deref_may_alias_ref_p_1 (arg, ref)) - return true; - } - else + /* If we don't have base pointer, give up. */ + if (!ref->ref && !ref->base) + continue; + + ao_ref ref2; + if (access_node.get_ao_ref (stmt, &ref2)) { - ao_ref ref2; - poly_offset_int off = (poly_offset_int)access_node->offset - + ((poly_offset_int)access_node->parm_offset - << LOG2_BITS_PER_UNIT); - poly_int64 off2; - if (off.to_shwi (&off2)) - { - ao_ref_init_from_ptr_and_range - (&ref2, arg, true, off2, - access_node->size, - access_node->max_size); - ref2.ref_alias_set = ref_set; - ref2.base_alias_set = base_set; - if (refs_may_alias_p_1 (&ref2, ref, tbaa_p)) - return true; - } - else if (ptr_deref_may_alias_ref_p_1 (arg, ref)) + ref2.ref_alias_set = ref_node->ref; + ref2.base_alias_set = base_node->base; + if (refs_may_alias_p_1 (&ref2, ref, tbaa_p)) return true; } + else if (ptr_deref_may_alias_ref_p_1 (arg, ref)) + return true; + num_tests++; } } diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c index 0e8c4ed1435..ce0083a6dab 100644 --- a/gcc/tree-ssa-dse.c +++ b/gcc/tree-ssa-dse.c @@ -1079,35 +1079,25 @@ dse_optimize_call (gimple_stmt_iterator *gsi, sbitmap live_bytes) if (!summary || !summary->try_dse) return false; - modref_base_node *base_node; - modref_ref_node *ref_node; - modref_access_node *access_node; - size_t i, j, k; bool by_clobber_p = false; /* Walk all memory writes and verify that they are dead. */ - FOR_EACH_VEC_SAFE_ELT (summary->stores->bases, i, base_node) - FOR_EACH_VEC_SAFE_ELT (base_node->refs, j, ref_node) - FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node) + for (auto base_node : summary->stores->bases) + for (auto ref_node : base_node->refs) + for (auto access_node : ref_node->accesses) { - gcc_checking_assert (access_node->parm_offset_known); + tree arg = access_node.get_call_arg (stmt); - tree arg; - if (access_node->parm_index == MODREF_STATIC_CHAIN_PARM) - arg = gimple_call_chain (stmt); - else - arg = gimple_call_arg (stmt, access_node->parm_index); + if (!arg) + return false; + + if (integer_zerop (arg) && flag_delete_null_pointer_checks) + continue; ao_ref ref; - poly_offset_int off = (poly_offset_int)access_node->offset - + ((poly_offset_int)access_node->parm_offset - << LOG2_BITS_PER_UNIT); - poly_int64 off2; - if (!off.to_shwi (&off2)) + + if (!access_node.get_ao_ref (stmt, &ref)) return false; - ao_ref_init_from_ptr_and_range - (&ref, arg, true, off2, access_node->size, - access_node->max_size); ref.ref_alias_set = ref_node->ref; ref.base_alias_set = base_node->base;