> 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). 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. 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 <hubi...@ucw.cz> * 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<unsigned char> &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 <gcall *> (stmt)) + maybe_warn_pass_by_reference (call, wlims); else if (gimple_assign_load_p (stmt) && gimple_has_location (stmt)) {