Re: Detect EAF flags in ipa-modref

[Jan Hubicka <hubicka@ucw.cz>]

> 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  <hubicka@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))
 	    {