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 03E203858D39 for ; Thu, 16 Mar 2023 12:05:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 03E203858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id DEC911FE03; Thu, 16 Mar 2023 12:05:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1678968356; 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=kOejFDv8YHhIaAUd0eZAqJA4eg9uVcGDhG7doQ66g5s=; b=w0TB1+y6SmTDP/wnxB3mHfh4cG1VFdbSdrwYufG62bP7Kf4zhy4a5QHeuIAeej1plGjQUZ LSvU+hRl8CjZwC2ba/Esh6WNAMWBfPeh5KS4vm611m4YkrAhY6g8WPA85Dq6y5byd28Djo /fBqWiOLLUoxmovccVf/gKppOul9h2U= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1678968356; 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=kOejFDv8YHhIaAUd0eZAqJA4eg9uVcGDhG7doQ66g5s=; b=M8N/5aXAkk5w4zzD6ddhqS9yU/DD3RZpz7q2RJepJpTtoSbENGXphQmDUyTSXPWqLGbdBM KQVuCuxBaaIWgmDg== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id B19332C141; Thu, 16 Mar 2023 12:05:56 +0000 (UTC) Date: Thu, 16 Mar 2023 12:05:56 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: Jan Hubicka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] tree-optimization/106912 - IPA profile and pure/const In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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,SPF_HELO_NONE,SPF_PASS,TXREP 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: On Thu, 16 Mar 2023, Jakub Jelinek wrote: > On Fri, Nov 25, 2022 at 09:26:34PM +0100, Richard Biener via Gcc-patches wrote: > > > We could > > > probably keep tract if any instrumented code was ever inlined into a > > > given function and perhaps just start ignoring attributes set on types? > > > > But ignoring attributes on types makes all indirect calls not > > const/pure annotatable (OK, they are not pure annotatable because of > > the above bug). I really don't see how to conservatively solve this > > issue? > > > Maybe we can ignore all pure/const when the cgraph state is > > in profile-instrumented state? Of course we have multiple "APIs" > > to query that. > > I think that's the way to go. But we'd need to arrange somewhere during IPA > to add vops to all those pure/const calls if -fprofile-generate (direct or > indirect; not sure what exact flags) and after that make sure all our APIs > don't return ECF_CONST, ECF_PURE or ECF_LOOPING_CONST_OR_PURE. > Could be e.g. > if (whatever) > flags &= ~(ECF_CONST | ECF_PURE | ECF_LOOPING_CONST_OR_PURE); > at the end of flags_from_decl_or_type, internal_fn_flags, what else? > Although, perhaps internal_fn_flags don't need to change, because internal > calls don't really have callees. > > Or is just ECF_CONST enough given that ECF_PURE is on fndecls and not > types? > > Or perhaps just start ignoring TYPE_READONLY -> ECF_CONST in > flags_from_decl_or_type if that flag is on? > > Is that rewriting currently what tree_profiling does in the > /* Update call statements and rebuild the cgraph. */ > FOR_EACH_DEFINED_FUNCTION (node) > spot where it calls update_stmt on all call statements? > > If so, could we just set that global? flag instead or in addition to > doing those node->set_const_flag (false, false); calls and > change flags_from_decl_or_type, plus I guess lto1 should set that > flag if it is global on start as well if > !flag_auto_profile > && (flag_branch_probabilities || flag_test_coverage || profile_arc_flag) > ? > > I think just ignoring ECF_CONST from TYPE_READONLY might be best, if we > have external functions like builtins from libc/libm and don't have their > bodies, we can still treat them as const, the only problem is with the > indirect calls where we don't know if we do or don't have a body for > the callees and whether we've instrumented those or not. I think we want something reflected on the IL. Because of LTO I think we cannot ignore externs (or we have to do massaging at stream-in). The following brute-force works for the testcase. I suppose since we leave const/pure set on functions without body in the compile-time TU (and ignore LTO there) we could do the same here. I also wonder if that whole function walk is necessary if not profile_arc_flag || flag_test_coverage ... Honza - does this look OK to you? I'll test guarding the whole outer loop with profile_arc_flag || flag_test_coverage separately. diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc index ea9d7a23443..c8789618f96 100644 --- a/gcc/tree-profile.cc +++ b/gcc/tree-profile.cc @@ -840,9 +840,29 @@ tree_profiling (void) gimple_stmt_iterator gsi; for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { - gimple *stmt = gsi_stmt (gsi); - if (is_gimple_call (stmt)) - update_stmt (stmt); + gcall *call = dyn_cast (gsi_stmt (gsi)); + if (!call) + continue; + + if (profile_arc_flag || flag_test_coverage) + { + /* We do not clear pure/const on decls without body. */ + tree fndecl = gimple_call_fndecl (call); + if (fndecl && !gimple_has_body_p (fndecl)) + continue; + + /* Drop the const attribute from the call type (the pure + attribute is not available on types). */ + tree fntype = gimple_call_fntype (call); + if (fntype && TYPE_READONLY (fntype)) + gimple_call_set_fntype (call, build_qualified_type + (fntype, (TYPE_QUALS (fntype) + & ~TYPE_QUAL_CONST))); + } + + /* Update virtual operands of calls to no longer const/pure + functions. */ + update_stmt (call); } }