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 10D6C385B1B2 for ; Fri, 25 Nov 2022 12:11:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 10D6C385B1B2 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=ucw.cz Authentication-Results: sourceware.org; spf=none smtp.mailfrom=kam.mff.cuni.cz Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 28D49284AE5; Fri, 25 Nov 2022 13:11:18 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1; t=1669378278; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7lbrP6KC48M0mc47aelFlFrvLSr31cXLu1PpcUWvdHY=; b=gU2Qe6Q779+dVFV48mFaIDDPZm3DbFHSsAT03ZjKXxdOkZpjnQpQ78UYA+j6z4/Mtq33Cm vgYFYC5Ibra+TqfbOPx+IYqpvRx858JWteRQ6+BZurbFHYFCAYkOHuwQy22fQ3/w6sXjuc PyoQHZgfs1rYZyQ495F3mWCskMrFhmg= Date: Fri, 25 Nov 2022 13:11:18 +0100 From: Jan Hubicka To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] tree-optimization/106912 - IPA profile and pure/const Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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: > > > > Am 25.11.2022 um 11:05 schrieb Jan Hubicka via Gcc-patches : > > > >  > >> > >> IPA profile instrumentation tries to clear the pure and const > >> flags of functions but that's quite hopeless in particular for > >> const since that attribute prevails on the type and thus on each > >> call to the function leading to inconsistencies in the IL and > >> eventual checking ICEs. There's no good reason to do this and > >> it wouldn't fixup any indirect calls so just don't. No other > >> instrumentation GCC does bothers about this. > > > > This was mostly meant to deadl with situation where we auto-detect > > function to be const and then partially inline it to a loop. > > Then both caller and callee accesses same counters in the memory and if > > you move load/stores out of the loop in caller you lose counters and get > > inconsistencies at profile read-in time. > > Don’t we Instrument after partial inlining now? As said, since we have the fntype on the call this doesn’t work anymore for const functions via attributes. Full inlining can cause problem already. So for code like do { if (__builtin_expect (test,1)) a+=foo (a); else a+=foo (b); } while (....); we may end up inlining one of the two invocation. Then caller and callee will modify the same counter. If we handle the remaining call as const, we can hoist the counter modifications out of the loop and mix them up. I remember I run into an actual example of this problem during GCC bootstrap. There the function was auto-detected to be const by early pure-const pass so type was not an problem. You are right we ought to do something about types since the scenario above can happen with foo being declared with an attribute as well. Honza > > Richard > > Honza > >> > >> Bootstrap and regtest pending on x86_64-unknown-linux-gnu, OK? > >> > >> Thanks, > >> Richard. > >> > >> PR tree-optimization/106912 > >> * tree-profile.cc (tree_profiling): Do not clear pure/const > >> flags. > >> > >> * gcc.dg/pr106912.c: New testcase. > >> --- > >> gcc/testsuite/gcc.dg/pr106912.c | 16 ++++++++++++++++ > >> gcc/tree-profile.cc | 3 --- > >> 2 files changed, 16 insertions(+), 3 deletions(-) > >> create mode 100644 gcc/testsuite/gcc.dg/pr106912.c > >> > >> diff --git a/gcc/testsuite/gcc.dg/pr106912.c b/gcc/testsuite/gcc.dg/pr106912.c > >> new file mode 100644 > >> index 00000000000..8faa877d8b3 > >> --- /dev/null > >> +++ b/gcc/testsuite/gcc.dg/pr106912.c > >> @@ -0,0 +1,16 @@ > >> +/* { dg-do compile } */ > >> +/* { dg-options "-O2 -fPIC -ftree-vectorize -fprofile-generate" } */ > >> + > >> +__attribute__ ((__simd__)) > >> +__attribute__ ((__nothrow__ , __leaf__ , __const__)) > >> +double foo (double x); > >> +void bar(double *f, int n) > >> +{ > >> + int i; > >> + for (i = 0; i < n; i++) > >> + f[i] = foo(f[i]); > >> +} > >> +double foo(double x) > >> +{ > >> + return x * x / 3.0; > >> +} > >> diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc > >> index 2beb49241f2..5491b398870 100644 > >> --- a/gcc/tree-profile.cc > >> +++ b/gcc/tree-profile.cc > >> @@ -814,9 +814,6 @@ tree_profiling (void) > >> /* Don't profile functions produced for builtin stuff. */ > >> if (DECL_SOURCE_LOCATION (node->decl) == BUILTINS_LOCATION) > >> continue; > >> - > >> - node->set_const_flag (false, false); > >> - node->set_pure_flag (false, false); > >> } > >> > >> /* Update call statements and rebuild the cgraph. */ > >> -- > >> 2.35.3