From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 5DCC53858C50 for ; Thu, 7 Apr 2022 12:25:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5DCC53858C50 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 513F41F859; Thu, 7 Apr 2022 12:25:02 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 3991313485; Thu, 7 Apr 2022 12:25:02 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id +JfrDB7YTmKeEAAAMHmgww (envelope-from ); Thu, 07 Apr 2022 12:25:02 +0000 Date: Thu, 7 Apr 2022 14:25:01 +0200 (CEST) From: Richard Biener To: Jan Hubicka cc: gcc-patches@gcc.gnu.org Subject: Re: Fix wrong code in gnatmake In-Reply-To: Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: Thu, 07 Apr 2022 12:25:06 -0000 On Thu, 7 Apr 2022, Jan Hubicka wrote: > Hi, > this patch fixes miscompilation of gnatmake. Modref attempts to track memory > accesses relative to the base pointers which are parameters of functions. > If it fails, it still makes difference between unknown memory access and > global memory access. The second makes it possible to disambiguate with > memory that is not accessible from outside world (i.e. everything that does > not escape from the caller function). This is useful so we do not punt > when unknown function is called. > > Now I added ref_may_access_global_memory_p to tree-ssa-alias whic is using > ptr_deref_may_alias_global_p. There is however a shift in meaning of this > predicate: the second tests that the dereference may alias with global variable. > > In the testcase we are disambiguating heap allocated escaping memory which is > not a global variable but it is still a global memory in the modref's sense. > So we need to test in addition contains_escaped. > > The patch simply copies logic from the predicate and adds the check. > I am not sure if there is better way to handle this? I'm testing the following variant which exposes this detail (escaped local memory global or not) in the APIs that say "global" which allows to remove ref_may_access_global_memory_p. Richard. >From 66f986c1bdea2597b1e9406429bf1c144d757a3e Mon Sep 17 00:00:00 2001 From: Richard Biener Date: Thu, 7 Apr 2022 14:07:54 +0200 Subject: [PATCH] ipa/104303 - miscompilation of gnatmake To: gcc-patches@gcc.gnu.org Modref attempts to track memory accesses relative to the base pointers which are parameters of functions. If it fails, it still makes difference between unknown memory access and global memory access. The second makes it possible to disambiguate with memory that is not accessible from outside world (i.e. everything that does not escape from the caller function). This is useful so we do not punt when unknown function is called. The added ref_may_access_global_memory_p ends up using ptr_deref_may_alias_global_p which does not consider escaped automatic variables as global. For modref those are still global since they can be accessed from functions called. The following adds a flag to the *_global_p APIs indicating whether escaped local memory should be considered as global or not and removes ref_may_access_global_memory_p in favor of using ref_may_alias_global_p with the flag set to true. Richard. 2022-04-07 Richard Biener Jan Hubicka PR ipa/104303 * tree-ssa-alias.h (ptr_deref_may_alias_global_p, ref_may_alias_global_p, ref_may_alias_global_p, stmt_may_clobber_global_p, pt_solution_includes_global): Add bool parameters indicating whether escaped locals should be considered global. * tree-ssa-structalias.cc (pt_solution_includes_global): When the new escaped_nonlocal_p flag is true also consider pt->vars_contains_escaped. * tree-ssa-alias.cc (ptr_deref_may_alias_global_p): Pass down new escaped_nonlocal_p flag. (ref_may_alias_global_p): Likewise. (stmt_may_clobber_global_p): Likewise. (ref_may_alias_global_p_1): Likewise. For decls also query the escaped solution if true. (ref_may_access_global_memory_p): Remove. (modref_may_conflict): Use ref_may_alias_global_p with escaped locals considered global. (ref_maybe_used_by_stmt_p): Adjust. * ipa-fnsummary.cc (points_to_local_or_readonly_memory_p): Likewise. * tree-ssa-dse.cc (dse_classify_store): Likewise. * trans-mem.cc (thread_private_new_memory): Likewise, but consider escaped locals global. * tree-ssa-dce.cc (mark_stmt_if_obviously_necessary): Likewise. * gnat.dg/concat5.adb: New. * gnat.dg/concat5_pkg1.adb: Likewise. * gnat.dg/concat5_pkg1.ads: Likewise. * gnat.dg/concat5_pkg2.adb: Likewise. * gnat.dg/concat5_pkg2.ads: Likewise. --- gcc/ipa-fnsummary.cc | 2 +- gcc/testsuite/gnat.dg/concat5.adb | 9 ++++ gcc/testsuite/gnat.dg/concat5_pkg1.adb | 18 +++++++ gcc/testsuite/gnat.dg/concat5_pkg1.ads | 5 ++ gcc/testsuite/gnat.dg/concat5_pkg2.adb | 10 ++++ gcc/testsuite/gnat.dg/concat5_pkg2.ads | 5 ++ gcc/trans-mem.cc | 2 +- gcc/tree-ssa-alias.cc | 65 ++++++++++---------------- gcc/tree-ssa-alias.h | 10 ++-- gcc/tree-ssa-dce.cc | 2 +- gcc/tree-ssa-dse.cc | 4 +- gcc/tree-ssa-structalias.cc | 15 ++++-- 12 files changed, 93 insertions(+), 54 deletions(-) create mode 100644 gcc/testsuite/gnat.dg/concat5.adb create mode 100644 gcc/testsuite/gnat.dg/concat5_pkg1.adb create mode 100644 gcc/testsuite/gnat.dg/concat5_pkg1.ads create mode 100644 gcc/testsuite/gnat.dg/concat5_pkg2.adb create mode 100644 gcc/testsuite/gnat.dg/concat5_pkg2.ads diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc index 10b679bbb27..8a19a54998b 100644 --- a/gcc/ipa-fnsummary.cc +++ b/gcc/ipa-fnsummary.cc @@ -2587,7 +2587,7 @@ points_to_local_or_readonly_memory_p (tree t) && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)) && t == ssa_default_def (cfun, DECL_RESULT (current_function_decl))) return true; - return !ptr_deref_may_alias_global_p (t); + return !ptr_deref_may_alias_global_p (t, false); } if (TREE_CODE (t) == ADDR_EXPR) return refs_local_or_readonly_memory_p (TREE_OPERAND (t, 0)); diff --git a/gcc/testsuite/gnat.dg/concat5.adb b/gcc/testsuite/gnat.dg/concat5.adb new file mode 100644 index 00000000000..fabf24865d6 --- /dev/null +++ b/gcc/testsuite/gnat.dg/concat5.adb @@ -0,0 +1,9 @@ +-- { dg-do run } +-- { dg-options "-O" } + +with Concat5_Pkg1; use Concat5_Pkg1; + +procedure Concat5 is +begin + Scan ("-RTS=none"); +end; diff --git a/gcc/testsuite/gnat.dg/concat5_pkg1.adb b/gcc/testsuite/gnat.dg/concat5_pkg1.adb new file mode 100644 index 00000000000..c32f5af314e --- /dev/null +++ b/gcc/testsuite/gnat.dg/concat5_pkg1.adb @@ -0,0 +1,18 @@ +with Concat5_Pkg2; use Concat5_Pkg2; + +package body Concat5_Pkg1 is + + procedure Make_Failed (S : String); + pragma No_Inline (Make_Failed); + + procedure Make_Failed (S : String) is + begin + Compare (S); + end; + + procedure Scan (S : String) is + begin + Make_Failed ("option " & S & " should start with '--'"); + end; + +end Concat5_Pkg1; diff --git a/gcc/testsuite/gnat.dg/concat5_pkg1.ads b/gcc/testsuite/gnat.dg/concat5_pkg1.ads new file mode 100644 index 00000000000..7f46d87f6e6 --- /dev/null +++ b/gcc/testsuite/gnat.dg/concat5_pkg1.ads @@ -0,0 +1,5 @@ +package Concat5_Pkg1 is + + procedure Scan (S : String); + +end Concat5_Pkg1; diff --git a/gcc/testsuite/gnat.dg/concat5_pkg2.adb b/gcc/testsuite/gnat.dg/concat5_pkg2.adb new file mode 100644 index 00000000000..98bd38826b2 --- /dev/null +++ b/gcc/testsuite/gnat.dg/concat5_pkg2.adb @@ -0,0 +1,10 @@ +package body Concat5_Pkg2 is + + procedure Compare (S : String) is + begin + if S /= "option -RTS=none should start with '--'" then + raise Program_Error; + end if; + end; + +end Concat5_Pkg2; diff --git a/gcc/testsuite/gnat.dg/concat5_pkg2.ads b/gcc/testsuite/gnat.dg/concat5_pkg2.ads new file mode 100644 index 00000000000..2931ffd5d5a --- /dev/null +++ b/gcc/testsuite/gnat.dg/concat5_pkg2.ads @@ -0,0 +1,5 @@ +package Concat5_Pkg2 is + + procedure Compare (S : String); + +end Concat5_Pkg2; diff --git a/gcc/trans-mem.cc b/gcc/trans-mem.cc index e9feac2321c..ae2921f808e 100644 --- a/gcc/trans-mem.cc +++ b/gcc/trans-mem.cc @@ -1400,7 +1400,7 @@ thread_private_new_memory (basic_block entry_block, tree x) /* Search DEF chain to find the original definition of this address. */ do { - if (ptr_deref_may_alias_global_p (x)) + if (ptr_deref_may_alias_global_p (x, true)) { /* Address escapes. This is not thread-private. */ retval = mem_non_local; diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc index 50bd47b31f3..063f1893851 100644 --- a/gcc/tree-ssa-alias.cc +++ b/gcc/tree-ssa-alias.cc @@ -210,10 +210,12 @@ dump_alias_stats (FILE *s) } -/* Return true, if dereferencing PTR may alias with a global variable. */ +/* Return true, if dereferencing PTR may alias with a global variable. + When ESCAPED_LOCAL_P is true escaped local memory is also considered + global. */ bool -ptr_deref_may_alias_global_p (tree ptr) +ptr_deref_may_alias_global_p (tree ptr, bool escaped_local_p) { struct ptr_info_def *pi; @@ -230,7 +232,7 @@ ptr_deref_may_alias_global_p (tree ptr) return true; /* ??? This does not use TBAA to prune globals ptr may not access. */ - return pt_solution_includes_global (&pi->pt); + return pt_solution_includes_global (&pi->pt, escaped_local_p); } /* Return true if dereferencing PTR may alias DECL. @@ -480,37 +482,44 @@ ptrs_compare_unequal (tree ptr1, tree ptr2) return false; } -/* Returns whether reference REF to BASE may refer to global memory. */ +/* Returns whether reference REF to BASE may refer to global memory. + When ESCAPED_LOCAL_P is true escaped local memory is also considered + global. */ static bool -ref_may_alias_global_p_1 (tree base) +ref_may_alias_global_p_1 (tree base, bool escaped_local_p) { if (DECL_P (base)) - return is_global_var (base); + return (is_global_var (base) + || (escaped_local_p + && pt_solution_includes (&cfun->gimple_df->escaped, base))); else if (TREE_CODE (base) == MEM_REF || TREE_CODE (base) == TARGET_MEM_REF) - return ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0)); + return ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0), + escaped_local_p); return true; } bool -ref_may_alias_global_p (ao_ref *ref) +ref_may_alias_global_p (ao_ref *ref, bool escaped_local_p) { tree base = ao_ref_base (ref); - return ref_may_alias_global_p_1 (base); + return ref_may_alias_global_p_1 (base, escaped_local_p); } bool -ref_may_alias_global_p (tree ref) +ref_may_alias_global_p (tree ref, bool escaped_local_p) { tree base = get_base_address (ref); - return ref_may_alias_global_p_1 (base); + return ref_may_alias_global_p_1 (base, escaped_local_p); } -/* Return true whether STMT may clobber global memory. */ +/* Return true whether STMT may clobber global memory. + When ESCAPED_LOCAL_P is true escaped local memory is also considered + global. */ bool -stmt_may_clobber_global_p (gimple *stmt) +stmt_may_clobber_global_p (gimple *stmt, bool escaped_local_p) { tree lhs; @@ -531,7 +540,7 @@ stmt_may_clobber_global_p (gimple *stmt) case GIMPLE_ASSIGN: lhs = gimple_assign_lhs (stmt); return (TREE_CODE (lhs) != SSA_NAME - && ref_may_alias_global_p (lhs)); + && ref_may_alias_global_p (lhs, escaped_local_p)); case GIMPLE_CALL: return true; default: @@ -2567,30 +2576,6 @@ refs_output_dependent_p (tree store1, tree store2) return refs_may_alias_p_1 (&r1, &r2, false); } -/* Return ture if REF may access global memory. */ - -bool -ref_may_access_global_memory_p (ao_ref *ref) -{ - if (!ref->ref) - return true; - tree base = ao_ref_base (ref); - if (TREE_CODE (base) == MEM_REF - || TREE_CODE (base) == TARGET_MEM_REF) - { - if (ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0))) - return true; - } - else - { - if (!auto_var_in_fn_p (base, current_function_decl) - || pt_solution_includes (&cfun->gimple_df->escaped, - base)) - return true; - } - return false; -} - /* Returns true if and only if REF may alias any access stored in TT. IF TBAA_P is true, use TBAA oracle. */ @@ -2654,7 +2639,7 @@ modref_may_conflict (const gcall *stmt, { if (global_memory_ok) continue; - if (ref_may_access_global_memory_p (ref)) + if (ref_may_alias_global_p (ref, true)) return true; global_memory_ok = true; num_tests++; @@ -2990,7 +2975,7 @@ ref_maybe_used_by_stmt_p (gimple *stmt, ao_ref *ref, bool tbaa_p) return is_global_var (base); else if (TREE_CODE (base) == MEM_REF || TREE_CODE (base) == TARGET_MEM_REF) - return ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0)); + return ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0), false); return false; } diff --git a/gcc/tree-ssa-alias.h b/gcc/tree-ssa-alias.h index dfb21275657..fa081ab0173 100644 --- a/gcc/tree-ssa-alias.h +++ b/gcc/tree-ssa-alias.h @@ -121,18 +121,18 @@ extern tree ao_ref_alias_ptr_type (ao_ref *); extern tree ao_ref_base_alias_ptr_type (ao_ref *); extern bool ao_ref_alignment (ao_ref *, unsigned int *, unsigned HOST_WIDE_INT *); -extern bool ptr_deref_may_alias_global_p (tree); +extern bool ptr_deref_may_alias_global_p (tree, bool); extern bool ptr_derefs_may_alias_p (tree, tree); extern bool ptrs_compare_unequal (tree, tree); -extern bool ref_may_alias_global_p (tree); -extern bool ref_may_alias_global_p (ao_ref *); +extern bool ref_may_alias_global_p (tree, bool); +extern bool ref_may_alias_global_p (ao_ref *, bool); extern bool refs_may_alias_p (tree, tree, bool = true); extern bool refs_may_alias_p_1 (ao_ref *, ao_ref *, bool); extern bool refs_anti_dependent_p (tree, tree); extern bool refs_output_dependent_p (tree, tree); extern bool ref_maybe_used_by_stmt_p (gimple *, tree, bool = true); extern bool ref_maybe_used_by_stmt_p (gimple *, ao_ref *, bool = true); -extern bool stmt_may_clobber_global_p (gimple *); +extern bool stmt_may_clobber_global_p (gimple *, bool); extern bool stmt_may_clobber_ref_p (gimple *, tree, bool = true); extern bool stmt_may_clobber_ref_p_1 (gimple *, ao_ref *, bool = true); extern bool call_may_clobber_ref_p (gcall *, tree, bool = true); @@ -171,7 +171,7 @@ extern void dump_alias_stats (FILE *); extern unsigned int compute_may_aliases (void); extern bool pt_solution_empty_p (const pt_solution *); extern bool pt_solution_singleton_or_null_p (struct pt_solution *, unsigned *); -extern bool pt_solution_includes_global (struct pt_solution *); +extern bool pt_solution_includes_global (struct pt_solution *, bool); extern bool pt_solution_includes (struct pt_solution *, const_tree); extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution *); extern void pt_solution_reset (struct pt_solution *); diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc index 2a13ea34829..34ce8abe33a 100644 --- a/gcc/tree-ssa-dce.cc +++ b/gcc/tree-ssa-dce.cc @@ -315,7 +315,7 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) } if ((gimple_vdef (stmt) && keep_all_vdefs_p ()) - || stmt_may_clobber_global_p (stmt)) + || stmt_may_clobber_global_p (stmt, true)) { mark_stmt_necessary (stmt, true); return; diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc index 3beaed3ad38..881a2d0f98d 100644 --- a/gcc/tree-ssa-dse.cc +++ b/gcc/tree-ssa-dse.cc @@ -1030,7 +1030,7 @@ dse_classify_store (ao_ref *ref, gimple *stmt, just pretend the stmt makes itself dead. Otherwise fail. */ if (defs.is_empty ()) { - if (ref_may_alias_global_p (ref)) + if (ref_may_alias_global_p (ref, false)) return DSE_STORE_LIVE; if (by_clobber_p) @@ -1062,7 +1062,7 @@ dse_classify_store (ao_ref *ref, gimple *stmt, { /* But if the store is to global memory it is definitely not dead. */ - if (ref_may_alias_global_p (ref)) + if (ref_may_alias_global_p (ref, false)) return DSE_STORE_LIVE; defs.unordered_remove (i); } diff --git a/gcc/tree-ssa-structalias.cc b/gcc/tree-ssa-structalias.cc index d318f883e70..581bdcf5652 100644 --- a/gcc/tree-ssa-structalias.cc +++ b/gcc/tree-ssa-structalias.cc @@ -6995,10 +6995,12 @@ pt_solution_singleton_or_null_p (struct pt_solution *pt, unsigned *uid) return true; } -/* Return true if the points-to solution *PT includes global memory. */ +/* Return true if the points-to solution *PT includes global memory. + If ESCAPED_LOCAL_P is true then escaped local variables are also + considered global. */ bool -pt_solution_includes_global (struct pt_solution *pt) +pt_solution_includes_global (struct pt_solution *pt, bool escaped_local_p) { if (pt->anything || pt->nonlocal @@ -7009,12 +7011,17 @@ pt_solution_includes_global (struct pt_solution *pt) || pt->vars_contains_escaped_heap) return true; + if (escaped_local_p && pt->vars_contains_escaped) + return true; + /* 'escaped' is also a placeholder so we have to look into it. */ if (pt->escaped) - return pt_solution_includes_global (&cfun->gimple_df->escaped); + return pt_solution_includes_global (&cfun->gimple_df->escaped, + escaped_local_p); if (pt->ipa_escaped) - return pt_solution_includes_global (&ipa_escaped_pt); + return pt_solution_includes_global (&ipa_escaped_pt, + escaped_local_p); return false; } -- 2.34.1