From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id E1D883858C2D for ; Mon, 20 Nov 2023 16:28:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E1D883858C2D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=suse.cz ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E1D883858C2D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:67c:2178:6::1c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700497733; cv=none; b=HvlV+udIOmsvciEUe1KCrPqH005RyOyvrL0i9lEdpvMjPUyLZiicoAFq/Ps+f5ijjpv+PBWebHGIG+1RL6IjKi76xUYdsCnB39aIEjfD1Ers/0h8xC7X6fJajE/q1JCxSDLz2vl4MaYDb0ZSQrAxNuF0LbNZ5fNxqpEjCPEusU0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700497733; c=relaxed/simple; bh=aik+n5JUQLh1pGm8hgZXnFAi2b8CIgIWJToZMIcrR70=; h=DKIM-Signature:DKIM-Signature:From:To:Subject:Date:Message-ID: MIME-Version; b=mCqo6dWMfBJ722qGkj+sUXacdXL0s9A9TvMn1iN2wLLyOAOS6d6QFDPd2/O8DIkmbbjWnrazlU0qtgHuLe3sJjQb4b8qmfYJUoQQrtvSsIVufdIHg8TnqKar8ZEXGHzJUzOANq0A0zNt0uhrF93WcoRVEAUES86dgNsNH3z70Nk= ARC-Authentication-Results: i=1; server2.sourceware.org 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-out1.suse.de (Postfix) with ESMTPS id DEB4821874; Mon, 20 Nov 2023 16:28:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1700497728; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=V1aWsNx+Xzvo2l0RPAQNi9sm+tt1ozLFJVgwUkf4osA=; b=C/RzWdxCbzlF6uo+WQTnIycTjsHpiEsoKK43moqPiTAUFM8/hi0ZH4JDt9TMHeZq7/oN4H 5CWQL4DYb+T0Olau9ZofyhptNfhesbsGan0HjVDJvI4RfeKVsNpbLqisJ/w4csJ+TVTdDv UQI+EspNCpxHJpwwhap/qVuMkk/INn8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1700497728; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=V1aWsNx+Xzvo2l0RPAQNi9sm+tt1ozLFJVgwUkf4osA=; b=MAiW+8l0IPA91uKSoGtNLazQob5QLYX4QnmfuqngHNMpN3ib5xxwiUMDk9X15P23JmWm3m rEEiEb3b2LOGSRAw== 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 C8D7413499; Mon, 20 Nov 2023 16:28:48 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id yOAOMUCJW2XQbwAAMHmgww (envelope-from ); Mon, 20 Nov 2023 16:28:48 +0000 From: Martin Jambor To: Jan Hubicka Cc: gcc-patches@gcc.gnu.org Subject: Re: Propagate value ranges of return values In-Reply-To: References: <871qcmkmgd.fsf@gentoo.org> User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/29.1 (x86_64-suse-linux-gnu) Date: Mon, 20 Nov 2023 17:28:42 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain Authentication-Results: smtp-out1.suse.de; none X-Spam-Level: X-Spam-Score: -2.10 X-Spamd-Result: default: False [-2.10 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; MIME_GOOD(-0.10)[text/plain]; DKIM_SIGNED(0.00)[suse.cz:s=susede2_rsa,suse.cz:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.20)[-1.000]; RCPT_COUNT_TWO(0.00)[2]; INVALID_MSGID(1.70)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; MID_RHS_NOT_FQDN(0.50)[]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,INVALID_MSGID,SPF_HELO_NONE,SPF_SOFTFAIL,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, thanks for working on this. On Sun, Nov 19 2023, Jan Hubicka wrote: > Hi, > this is updated version which also adds testuiste compensation > I lost earlier while maintaining the patch in my testing tree. > There are quite few testcases that use constant return values to hide > something from optimizer. > > Bootstrapped/regtested x86_64-linux. > gcc/ChangeLog: > > * cgraph.cc (add_detected_attribute_1): New function. > (cgraph_node::add_detected_attribute): Likewise. > * cgraph.h (cgraph_node::add_detected_attribute): Declare. > * common.opt: Add -Wsuggest-attribute=returns_nonnull. > * doc/invoke.texi: Document new flag. > * gimple-range-fold.cc (fold_using_range::range_of_call): > Use known reutrn value ranges. > * ipa-prop.cc (struct ipa_return_value_summary): New type. > (class ipa_return_value_sum_t): New type. > (ipa_return_value_sum): New summary. > (ipa_record_return_value_range): New function. > (ipa_return_value_range): New function. > * ipa-prop.h (ipa_return_value_range): Declare. > (ipa_record_return_value_range): Declare. > * ipa-pure-const.cc (warn_function_returns_nonnull): New funcion. > * ipa-utils.h (warn_function_returns_nonnull): Declare. > * symbol-summary.h: Fix comment. > * tree-vrp.cc (execute_ranger_vrp): Record return values. > > gcc/testsuite/ChangeLog: > > * g++.dg/ipa/devirt-2.C: Add noipa attribute to prevent ipa-vrp. > * g++.dg/ipa/devirt-7.C: Disable ipa-vrp. > * g++.dg/ipa/ipa-icf-2.C: Disable ipa-vrp. > * g++.dg/ipa/ipa-icf-3.C: Disable ipa-vrp. > * g++.dg/ipa/ivinline-1.C: Disable ipa-vrp. > * g++.dg/ipa/ivinline-3.C: Disable ipa-vrp. > * g++.dg/ipa/ivinline-5.C: Disable ipa-vrp. > * g++.dg/ipa/ivinline-8.C: Disable ipa-vrp. > * g++.dg/ipa/nothrow-1.C: Disable ipa-vrp. > * g++.dg/ipa/pure-const-1.C: Disable ipa-vrp. > * g++.dg/ipa/pure-const-2.C: Disable ipa-vrp. > * g++.dg/lto/inline-crossmodule-1_0.C: Disable ipa-vrp. > * gcc.c-torture/compile/pr106433.c: Add noipa attribute to prevent ipa-vrp. > * gcc.c-torture/execute/frame-address.c: Likewise. > * gcc.dg/ipa/fopt-info-inline-1.c: Disable ipa-vrp. > * gcc.dg/ipa/ipa-icf-25.c: Disable ipa-vrp. > * gcc.dg/ipa/ipa-icf-38.c: Disable ipa-vrp. > * gcc.dg/ipa/pure-const-1.c: Disable ipa-vrp. > * gcc.dg/ipa/remref-0.c: Add noipa attribute to prevent ipa-vrp. > * gcc.dg/tree-prof/time-profiler-1.c: Disable ipa-vrp. > * gcc.dg/tree-prof/time-profiler-2.c: Disable ipa-vrp. > * gcc.dg/tree-ssa/pr110269.c: Disable ipa-vrp. > * gcc.dg/tree-ssa/pr20701.c: Disable ipa-vrp. > * gcc.dg/tree-ssa/vrp05.c: Disable ipa-vrp. > * gcc.dg/tree-ssa/return-value-range-1.c: New test. > > diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc > index e41e5ad3ae7..71dacf23ce1 100644 > --- a/gcc/cgraph.cc > +++ b/gcc/cgraph.cc > @@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p) > return changed; > } > > +/* Worker to set malloc flag. */ I think the comment must be stale, and the name of the function also, it does not add anything, does it? > +static void > +add_detected_attribute_1 (cgraph_node *node, const char *attr, bool *changed) > +{ > + if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl))) > + { > + DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr), > + NULL_TREE, DECL_ATTRIBUTES (node->decl)); > + *changed = true; > + } > + > + ipa_ref *ref; > + FOR_EACH_ALIAS (node, ref) > + { > + cgraph_node *alias = dyn_cast (ref->referring); > + if (alias->get_availability () > AVAIL_INTERPOSABLE) > + add_detected_attribute_1 (alias, attr, changed); > + } > + > + for (cgraph_edge *e = node->callers; e; e = e->next_caller) > + if (e->caller->thunk > + && (e->caller->get_availability () > AVAIL_INTERPOSABLE)) > + add_detected_attribute_1 (e->caller, attr, changed); > +} > + > +/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any. */ Likewise. > + > +bool > +cgraph_node::add_detected_attribute (const char *attr) > +{ > + bool changed = false; > + > + if (get_availability () > AVAIL_INTERPOSABLE) > + add_detected_attribute_1 (this, attr, &changed); > + else > + { > + ipa_ref *ref; > + > + FOR_EACH_ALIAS (this, ref) > + { > + cgraph_node *alias = dyn_cast (ref->referring); > + if (alias->get_availability () > AVAIL_INTERPOSABLE) > + add_detected_attribute_1 (alias, attr, &changed); > + } > + } > + return changed; > +} > + > /* Worker to set noreturng flag. */ > static void > set_noreturn_flag_1 (cgraph_node *node, bool noreturn_p, bool *changed) [...] > diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc > index 6e9530c3d7f..998b7608d78 100644 > --- a/gcc/gimple-range-fold.cc > +++ b/gcc/gimple-range-fold.cc > @@ -44,6 +44,11 @@ along with GCC; see the file COPYING3. If not see > #include "value-query.h" > #include "gimple-range-op.h" > #include "gimple-range.h" > +#include "cgraph.h" > +#include "alloc-pool.h" > +#include "symbol-summary.h" > +#include "ipa-utils.h" > +#include "ipa-prop.h" > // Construct a fur_source, and set the m_query field. > > fur_source::fur_source (range_query *q) > @@ -1013,6 +1018,25 @@ fold_using_range::range_of_call (vrange &r, gcall *call, fur_source &) > else > r.set_varying (type); > > + tree callee = gimple_call_fndecl (call); > + if (callee > + && useless_type_conversion_p (TREE_TYPE (TREE_TYPE (callee)), type)) > + { > + Value_Range val; > + if (ipa_return_value_range (val, callee)) > + { > + r.intersect (val); > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, "Using return value range of "); > + print_generic_expr (dump_file, callee, TDF_SLIM); > + fprintf (dump_file, ": "); > + val.dump (dump_file); > + fprintf (dump_file, "\n"); > + } > + } > + } > + > // If there is an LHS, intersect that with what is known. > if (lhs) > { > diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > index 7de2b788185..e77bc9c340b 100644 > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > @@ -237,6 +237,35 @@ gt_ggc_mx (ipa_vr *&x) > return gt_ggc_mx ((ipa_vr *) x); > } > > +/* Analysis summery of function call return value. */ > +struct GTY(()) ipa_return_value_summary > +{ > + /* Known value range. > + This needs to be wrapped in struccture due to specific way structure. > + we allocate ipa_vr. */ > + ipa_vr *vr; > +}; > + > +/* Function summary for return values. */ > +class ipa_return_value_sum_t : public function_summary > +{ > +public: > + ipa_return_value_sum_t (symbol_table *table, bool ggc): > + function_summary (table, ggc) { } > + > + /* Hook that is called by summary when a node is duplicated. */ > + void duplicate (cgraph_node *, > + cgraph_node *, > + ipa_return_value_summary *data, > + ipa_return_value_summary *data2) final override > + { > + *data2=*data; > + } > +}; > + > +/* Variable hoding the return value summary. */ > +static GTY(()) function_summary *ipa_return_value_sum; > + > > /* Return true if DECL_FUNCTION_SPECIFIC_OPTIMIZATION of the decl associated > with NODE should prevent us from analyzing it for the purposes of IPA-CP. */ > @@ -5915,5 +5944,49 @@ ipcp_transform_function (struct cgraph_node *node) > return modified_mem_access ? TODO_update_ssa_only_virtuals : 0; > } > > +/* Record that current function return value range is VAL. */ > + > +void > +ipa_record_return_value_range (Value_Range val) > +{ > + cgraph_node *n = cgraph_node::get (current_function_decl); > + if (!ipa_return_value_sum) > + { > + if (!ipa_vr_hash_table) > + ipa_vr_hash_table = hash_table::create_ggc (37); > + ipa_return_value_sum = new (ggc_alloc_no_dtor ()) > + ipa_return_value_sum_t (symtab, true); > + ipa_return_value_sum->disable_insertion_hook (); > + } > + ipa_return_value_sum->get_create (n)->vr = ipa_get_value_range (val); > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, "Recording return range "); > + val.dump (dump_file); > + fprintf (dump_file, "\n"); > + } > +} > + > +/* Return true if value range of DECL is known and if so initialize > RANGE. */ Perhaps better: "If value range of the value returned by function DECL is known, return true and set RANGE to it." But I guess it does not matter much. > + > +bool > +ipa_return_value_range (Value_Range &range, tree decl) > +{ > + cgraph_node *n = cgraph_node::get (decl); > + if (!n || !ipa_return_value_sum) > + return false; > + enum availability avail; > + n = n->ultimate_alias_target (&avail); > + if (avail < AVAIL_AVAILABLE) > + return false; > + if (n->decl != decl && !useless_type_conversion_p (TREE_TYPE (decl), TREE_TYPE (n->decl))) > + return false; > + ipa_return_value_summary *v = ipa_return_value_sum->get (n); > + if (!v) > + return false; > + v->vr->get_vrange (range); > + return true; > +} > + > > #include "gt-ipa-prop.h" > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index fcd0e5c638f..5901c805c40 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -309,7 +309,7 @@ public: > void get_vrange (Value_Range &) const; > bool equal_p (const vrange &) const; > const vrange_storage *storage () const { return m_storage; } > - void streamer_read (lto_input_block *, data_in *); > + void streamer_read (lto_input_block *, class data_in *); Is this an intended change? Or is there some other hunk for streamer_read missing? > void streamer_write (output_block *) const; > void dump (FILE *) const; > > @@ -1274,4 +1274,7 @@ ipa_range_set_and_normalize (vrange &r, tree val) > r.set (val, val); > } > > +bool ipa_return_value_range (Value_Range &range, tree decl); > +void ipa_record_return_value_range (Value_Range val); > + > #endif /* IPA_PROP_H */ [...] > diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc > index 917fa873714..82001eff20e 100644 > --- a/gcc/tree-vrp.cc > +++ b/gcc/tree-vrp.cc > @@ -52,6 +52,12 @@ along with GCC; see the file COPYING3. If not see > #include "gimple-fold.h" > #include "tree-dfa.h" > #include "tree-ssa-dce.h" > +#include "alloc-pool.h" > +#include "cgraph.h" > +#include "symbol-summary.h" > +#include "ipa-utils.h" > +#include "ipa-prop.h" > +#include "attribs.h" > > // This class is utilized by VRP and ranger to remove __builtin_unreachable > // calls, and reflect any resulting global ranges. > @@ -1081,6 +1087,51 @@ execute_ranger_vrp (struct function *fun, bool warn_array_bounds_p, > array_checker.check (); > } > > + > + if (Value_Range::supports_type_p (TREE_TYPE > + (TREE_TYPE (current_function_decl))) > + && flag_ipa_vrp > + && !lookup_attribute ("noipa", DECL_ATTRIBUTES (current_function_decl))) > + { > + edge e; > + edge_iterator ei; > + bool found = false; > + Value_Range return_range (TREE_TYPE (TREE_TYPE (current_function_decl))); > + FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds) > + if (greturn *ret = dyn_cast (*gsi_last_bb (e->src))) > + { > + tree retval = gimple_return_retval (ret); > + if (!retval) > + { > + return_range.set_varying (TREE_TYPE (TREE_TYPE (current_function_decl))); > + found = true; > + continue; > + } > + Value_Range r (TREE_TYPE (retval)); > + if (ranger->range_of_expr (r, retval, ret) > + && !r.undefined_p () > + && !r.varying_p ()) > + { > + if (!found) > + return_range = r; > + else > + return_range.union_ (r); > + } > + else > + return_range.set_varying (TREE_TYPE (retval)); > + found = true; > + } > + if (found && !return_range.varying_p ()) > + { > + ipa_record_return_value_range (return_range); > + if (POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))) > + && return_range.nonzero_p () > + && cgraph_node::get (current_function_decl) > + ->add_detected_attribute ("returns_nonnull")) > + warn_function_returns_nonnull (current_function_decl); > + } > + } > + > phi_analysis_finalize (); > disable_ranger (fun); > scev_finalize (); Otherwise it looks great. Thanks, Martin