ipa-modref: merge flags when adding escape

Alexandre Oliva oliva@adacore.com
Fri Jun 18 09:09:12 GMT 2021


While working on some function splitting changes, I've got a
miscompilation in stagefeedback that I've tracked down to a
complicated scenario:

- ipa-modref miscomputes a function parameter as having EAF_DIRECT,
  because it's dereferenced and passed on to another function, but
  add_escape_point does not update the flags for the dereferenced
  SSA_NAME passed as a parameter, and the EAF_UNUSED in the value that
  first initializes it, that remains unchanged throughout, causes
  deref_flags to set EAF_DIRECT, among other flags.

- structalias, seeing the EAF_DIRECT in the parameter for that
  function, refrains from mak[ing]_transitive_closure_constraints for
  a pointer passed in that parameter.

- tree dse2 concludes the initializer of the pointed-to variable is a
  dead store and removes it.

The test depends on gimple passes's processing of functions in a
certain order to expose parm flag miscomputed by ipa-modref.  A
different order may enable the non-ipa modref2 pass to compute flags
differently and avoid the problem.

I've arranged for add_escape_point to merge flags, as the non-ipa path
does.  I've also caught and fixed an error in the dumping of escaping
flags.

The problem affects at least trunk and gcc-11.  I've so far bootstrapped
GCC 11, and I'm now regstrapping trunk.  Ok to install if it passes?


for  gcc/ChangeLog

	* ipa-modref.c (modref_lattice::add_escape_point): Merge
	min_flags into flags.
	(modref_lattice::dump): Fix escape_point's min_flags dumping.

for  gcc/testsuite/ChangeLog

	* c-c++-common/modref-dse.c: New.
---
 gcc/ipa-modref.c                        |    4 ++-
 gcc/testsuite/c-c++-common/modref-dse.c |   38 +++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/modref-dse.c

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index d5a8332fb5559..3b0830cb8759c 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -1392,7 +1392,7 @@ modref_lattice::dump (FILE *out, int indent) const
 	  fprintf (out, "%*s  Arg %i (%s) min flags", indent, "",
 		   escape_points[i].arg,
 		   escape_points[i].direct ? "direct" : "indirect");
-	  dump_eaf_flags (out, flags, false);
+	  dump_eaf_flags (out, escape_points[i].min_flags, false);
 	  fprintf (out, " in call ");
 	  print_gimple_stmt (out, escape_points[i].call, 0);
 	}
@@ -1411,7 +1411,7 @@ modref_lattice::add_escape_point (gcall *call, int arg, int min_flags,
 
   /* If we already determined flags to be bad enough,
    * we do not need to record.  */
-  if ((flags & min_flags) == flags)
+  if (!merge (min_flags))
     return false;
 
   FOR_EACH_VEC_ELT (escape_points, i, ep)
diff --git a/gcc/testsuite/c-c++-common/modref-dse.c b/gcc/testsuite/c-c++-common/modref-dse.c
new file mode 100644
index 0000000000000..5f64e8f4b5942
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/modref-dse.c
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse2-details" } */
+/* { dg-final { scan-tree-dump-not "Deleted dead store" "dse2" } } */
+
+struct foo { unsigned long bar; };
+
+unsigned y;
+
+static int __attribute__ ((__noinline__, __noclone__))
+wrapped (struct foo *p, int i);
+
+static int wrapper (struct foo *p);
+
+static int __attribute__ ((__noclone__))
+wrapper (struct foo *p) {
+  return wrapped (p, 1);
+}
+
+static int __attribute__ ((__noinline__, __noclone__))
+dind (struct foo **pp);
+
+int __attribute__ ((__noclone__, __no_reorder__))
+xfn () {
+  struct foo x = { 0xBADC0FFE };
+  struct foo *p = &x;
+  return dind (&p);
+}
+
+static int __attribute__ ((__noinline__, __no_reorder__))
+wrapped (struct foo *p, int i) {
+  return p->bar + i == y++;
+}
+
+static int __attribute__ ((__noinline__, __noclone__, __no_reorder__))
+dind (struct foo **pp) {
+  wrapper (*pp);
+  return 0;
+}


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>


More information about the Gcc-patches mailing list