From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 704D4385E83A; Mon, 16 Nov 2020 12:36:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 704D4385E83A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 58D04AC2D; Mon, 16 Nov 2020 12:36:36 +0000 (UTC) Date: Mon, 16 Nov 2020 13:36:36 +0100 (CET) From: Richard Biener Sender: rguenther@c653.arch.suse.de To: Jan Hubicka cc: Andreas Schwab , gcc@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: Detect EAF flags in ipa-modref In-Reply-To: <20201116105936.GB91562@kam.mff.cuni.cz> Message-ID: References: <20201110143143.GA91644@kam.mff.cuni.cz> <87361ahph7.fsf@linux-m68k.org> <20201115111254.GB45304@kam.mff.cuni.cz> <20201115123307.GC45304@kam.mff.cuni.cz> <20201115130351.GD45304@kam.mff.cuni.cz> <87k0umvbo2.fsf@igel.home> <20201115180710.GA86620@kam.mff.cuni.cz> <87k0ulbqk7.fsf@igel.home> <20201116105936.GB91562@kam.mff.cuni.cz> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Nov 2020 12:36:39 -0000 On Mon, 16 Nov 2020, Jan Hubicka wrote: > > On Nov 15 2020, Jan Hubicka wrote: > > > > >> See PR97840. > > > Thanks, > > > this is a false positive where we fail to discover that pointed-to type > > > is empty on non-x86_64 targets. This is triggered by better alias > > > analysis caused by non-escape discovery. > > > > > > While this is not a full fix (I hope someone with more experience on > > > C++ types and warnings will set up) I think it may be useful to avoid > > > warning on unused parameter. > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > That doesn't fix anything. > > It does silence the warning if you remove inline from function > declaration (as creduce while minimizing the testcase - the minimized > testcase was undefined that is why I did not include it at the end). > I now implemented one by hand. > > The reason is that gimple_call_arg_flags clears EAF_UNUSED > on symbols that !binds_to_current_def_p because we are worried that > symbol will be interposed by different version of the body with > same semantics that will actually read the arg. > This is bit paranoid check since we optimize things like *a == *a early > but with clang *a will trap if a==NULL. > > Richi, I think we can add "safe" parameter to gimple_call_arg_flags and > bypass this logic when we use it for warnings only (having body that > does not use the value is quite strong hint that it is unused by the > function). Eh, please not. > > I played with bit more testcases and found that we also want to disable > warning for const functions and sometimes EAF_UNUSED flag is dropped > becaue of clobber that is not necessary to do. If function only clobber > the target it can be considered unused past inlining. > > I am testing this improved patch and plan to commit if there are no > complains, but still we need to handle binds_to_current_def. > > On the other direction, Martin, I think we may also warn for args > that are !EAF_UNUSED and !EAF_NOCLOBBER. This will catch some cases > where user did not add "const" specifier to the declaration but > parameter is detected to be readonly. > > I also noticed that we do not detect EAF_UNUSED for fully unused > parameters (with no SSA names associated with them). I will fix that > incrementally. Make sure to not apply it based on that reason to aggregates ;) > Honza > > PR middle-end/97840 > * ipa-modref.c (analyze_ssa_name_flags): Skip clobbers if inlining > is done. > * tree-ssa-uninit.c (maybe_warn_pass_by_reference): Make stmt gcall; > skip const calls and unused arguments. > (warn_uninitialized_vars): Update prototype. > > gcc/testsuite/ChangeLog: > > 2020-11-16 Jan Hubicka > > * g++.dg/warn/unit-2.C: New test. > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index 4a43c50aa66..08fcc36a321 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1333,7 +1331,14 @@ analyze_ssa_name_flags (tree name, vec &known_flags, int depth) > /* Handle *name = exp. */ > else if (assign > && memory_access_to (gimple_assign_lhs (assign), name)) > - flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + { > + /* In general we can not ignore clobbers because they are > + barriers for code motion, however after inlining it is safe to > + do because local optimization passes do not consider clobbers > + from other functions. Similar logic is in ipa-pure-const.c. */ > + if (!cfun->after_inlining && !gimple_clobber_p (assign)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + } > /* ASM statements etc. */ > else if (!assign) > { > diff --git a/gcc/testsuite/g++.dg/warn/unit-2.C b/gcc/testsuite/g++.dg/warn/unit-2.C > new file mode 100644 > index 00000000000..30f3ceae191 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/unit-2.C > @@ -0,0 +1,29 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -Wmaybe-uninitialized" } */ > +struct a {int a;}; > +__attribute__ ((noinline)) > +void > +nowarn (const struct a *ptr) > +{ > + if (ptr) > + asm volatile (""); > +} > +void > +test() > +{ > + struct a ptr; > + nowarn (&ptr); > +} > +__attribute__ ((noinline)) > +int > +nowarn2 (const struct a *ptr, const struct a ptr2) > +{ > + return ptr != 0 || ptr2.a; > +} > +int mem; > +int > +test2() > +{ > + struct a ptr,ptr2={0}; > + return nowarn2 (&ptr, ptr2); > +} > diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c > index f23514395e0..c94831bfb75 100644 > --- a/gcc/tree-ssa-uninit.c > +++ b/gcc/tree-ssa-uninit.c > @@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs, > access implying read access to those objects. */ > > static void > -maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > +maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims) > { > if (!wlims.wmaybe_uninit) > return; > @@ -457,6 +457,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > if (!fntype) > return; > > + /* Const function do not read their arguments. */ > + if (gimple_call_flags (stmt) & ECF_CONST) > + return; > + > const built_in_function fncode > = (fndecl && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL) > ? DECL_FUNCTION_CODE (fndecl) : (built_in_function)BUILT_IN_LAST); > @@ -523,6 +527,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > (but not definitive) read access. */ > wlims.always_executed = false; > > + /* Ignore args we are not going to read from. */ > + if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED) > + continue; > + > tree arg = gimple_call_arg (stmt, argno - 1); > > ao_ref ref; > @@ -639,8 +647,8 @@ warn_uninitialized_vars (bool wmaybe_uninit) > if (gimple_vdef (stmt)) > wlims.vdef_cnt++; > > - if (is_gimple_call (stmt)) > - maybe_warn_pass_by_reference (stmt, wlims); > + if (gcall *call = dyn_cast (stmt)) > + maybe_warn_pass_by_reference (call, wlims); > else if (gimple_assign_load_p (stmt) > && gimple_has_location (stmt)) > { > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imend