Fix more ICEs in -fsave-optimization-record (PR tree-optimization/89235)

PR tree-optimization/89235 reports an ICE inside -fsave-optimization-record
whilst reporting the inlining chain of of the location_t in the
vect_location global.

This is very similar to PR tree-optimization/86637, fixed in r266821.

The issue is that the inlining chains are read from the location_t's
ad-hoc data, referencing GC-managed tree blocks, but the former are
not GC roots; it's simply assumed that old locations referencing dead
blocks never get used again.

The fix is to reset the "vect_location" global in more places.  Given
that is a somewhat subtle detail, the patch adds a sentinel class to
reset vect_location at the end of a scope.  Doing it as a class
simplifies the task of ensuring that the global is reset on every
exit path from a function, and also gives a good place to signpost
the above subtlety (in the documentation for the class).

The patch also adds test cases for both of the PRs mentioned above.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/testsuite/ChangeLog:
	PR tree-optimization/86637
	PR tree-optimization/89235
	* gcc.c-torture/compile/pr86637-1.c: New test.
	* gcc.c-torture/compile/pr86637-2.c: New test.
	* gcc.c-torture/compile/pr86637-3.c: New test.
	* gcc.c-torture/compile/pr89235.c: New test.

gcc/ChangeLog:
	PR tree-optimization/86637
	PR tree-optimization/89235
	* tree-vect-loop.c (optimize_mask_stores): Add an
	auto_purge_vect_location sentinel to ensure that vect_location is
	purged on exit.
	* tree-vectorizer.c
	(auto_purge_vect_location::~auto_purge_vect_location): New dtor.
	(try_vectorize_loop_1): Add an auto_purge_vect_location sentinel
	to ensure that vect_location is purged on exit.
	(pass_slp_vectorize::execute): Likewise, replacing the manual
	reset.
	* tree-vectorizer.h (class auto_purge_vect_location): New class.
---
 gcc/testsuite/gcc.c-torture/compile/pr86637-1.c |  13 +++
 gcc/testsuite/gcc.c-torture/compile/pr86637-2.c | 128 ++++++++++++++++++++++++
 gcc/testsuite/gcc.c-torture/compile/pr86637-3.c |  11 ++
 gcc/testsuite/gcc.c-torture/compile/pr89235.c   |  57 +++++++++++
 gcc/tree-vect-loop.c                            |   1 +
 gcc/tree-vectorizer.c                           |  13 ++-
 gcc/tree-vectorizer.h                           |  18 ++++
 7 files changed, 239 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr86637-1.c
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr86637-2.c
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr86637-3.c
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr89235.c

On February 7, 2019 9:10:15 PM GMT+01:00, David Malcolm <dmalcolm@redhat.com> wrote:
>PR tree-optimization/89235 reports an ICE inside
>-fsave-optimization-record
>whilst reporting the inlining chain of of the location_t in the
>vect_location global.
>
>This is very similar to PR tree-optimization/86637, fixed in r266821.
>
>The issue is that the inlining chains are read from the location_t's
>ad-hoc data, referencing GC-managed tree blocks, but the former are
>not GC roots; it's simply assumed that old locations referencing dead
>blocks never get used again.
>
>The fix is to reset the "vect_location" global in more places.  Given
>that is a somewhat subtle detail, the patch adds a sentinel class to
>reset vect_location at the end of a scope.  Doing it as a class
>simplifies the task of ensuring that the global is reset on every
>exit path from a function, and also gives a good place to signpost
>the above subtlety (in the documentation for the class).
>
>The patch also adds test cases for both of the PRs mentioned above.
>
>Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
>OK for trunk?

OK. 

Richard. 

>gcc/testsuite/ChangeLog:
>	PR tree-optimization/86637
>	PR tree-optimization/89235
>	* gcc.c-torture/compile/pr86637-1.c: New test.
>	* gcc.c-torture/compile/pr86637-2.c: New test.
>	* gcc.c-torture/compile/pr86637-3.c: New test.
>	* gcc.c-torture/compile/pr89235.c: New test.
>
>gcc/ChangeLog:
>	PR tree-optimization/86637
>	PR tree-optimization/89235
>	* tree-vect-loop.c (optimize_mask_stores): Add an
>	auto_purge_vect_location sentinel to ensure that vect_location is
>	purged on exit.
>	* tree-vectorizer.c
>	(auto_purge_vect_location::~auto_purge_vect_location): New dtor.
>	(try_vectorize_loop_1): Add an auto_purge_vect_location sentinel
>	to ensure that vect_location is purged on exit.
>	(pass_slp_vectorize::execute): Likewise, replacing the manual
>	reset.
>	* tree-vectorizer.h (class auto_purge_vect_location): New class.
>---
> gcc/testsuite/gcc.c-torture/compile/pr86637-1.c |  13 +++
>gcc/testsuite/gcc.c-torture/compile/pr86637-2.c | 128
>++++++++++++++++++++++++
> gcc/testsuite/gcc.c-torture/compile/pr86637-3.c |  11 ++
> gcc/testsuite/gcc.c-torture/compile/pr89235.c   |  57 +++++++++++
> gcc/tree-vect-loop.c                            |   1 +
> gcc/tree-vectorizer.c                           |  13 ++-
> gcc/tree-vectorizer.h                           |  18 ++++
> 7 files changed, 239 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr86637-1.c
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr86637-2.c
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr86637-3.c
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr89235.c
>
>diff --git a/gcc/testsuite/gcc.c-torture/compile/pr86637-1.c
>b/gcc/testsuite/gcc.c-torture/compile/pr86637-1.c
>new file mode 100644
>index 0000000..61b6381
>--- /dev/null
>+++ b/gcc/testsuite/gcc.c-torture/compile/pr86637-1.c
>@@ -0,0 +1,13 @@
>+/* { dg-options "-fsave-optimization-record -ftree-slp-vectorize
>--param ggc-min-expand=1 --param ggc-min-heapsize=1024" } */
>+
>+void
>+en (void)
>+{
>+}
>+
>+void
>+n4 (int zb)
>+{
>+  while (zb < 1)
>+    ++zb;
>+}
>diff --git a/gcc/testsuite/gcc.c-torture/compile/pr86637-2.c
>b/gcc/testsuite/gcc.c-torture/compile/pr86637-2.c
>new file mode 100644
>index 0000000..3b675ea
>--- /dev/null
>+++ b/gcc/testsuite/gcc.c-torture/compile/pr86637-2.c
>@@ -0,0 +1,128 @@
>+/* { dg-do compile { target fgraphite } } */
>+/* { dg-options "-floop-parallelize-all -fsave-optimization-record
>-ftree-parallelize-loops=2 -ftree-slp-vectorize" } */
>+
>+#include <stdint.h>
>+#include <stdlib.h>
>+
>+signed char qq;
>+int rm, mu, l9;
>+long long unsigned int ip;
>+
>+void
>+fi (void)
>+{
>+}
>+
>+void
>+il (long long unsigned int c5)
>+{
>+  int *na = &mu;
>+
>+  while (l9 < 1)
>+    {
>+      if (qq < 1)
>+        {
>+          short int ds = 0;
>+
>+          if ((ip - *na - ip / c5 != 0) && (*na / ((c5 + c5) && !!ip)
>!= 0))
>+            __builtin_trap ();
>+
>+          rm = -1 / (!!rm && !!c5);
>+
>+          while (qq < 1)
>+            {
>+              ++*na;
>+              ++ip;
>+              if (*na < (int) ip)
>+                while (ds < 2)
>+                  {
>+                    ++qq;
>+                    ++ds;
>+                  }
>+            }
>+        }
>+
>+      ++l9;
>+    }
>+
>+  for (;;)
>+    {
>+    }
>+}
>+
>+void
>+uu (short int wk)
>+{
>+  int64_t tl = ip;
>+
>+  while (ip < 2)
>+    {
>+      signed char vn;
>+
>+      for (vn = 1; vn != 0; ++vn)
>+        {
>+          int rh;
>+
>+          if (qq < 1)
>+            {
>+              while (mu < 1)
>+                ip = 0;
>+
>+              while (rm != 0)
>+                ++rm;
>+            }
>+
>+          for (rh = 0; rh < 3; ++rh)
>+            {
>+              int ab;
>+
>+              for (ab = 0; ab < 5; ++ab)
>+                {
>+                  tl -= qq;
>+                  wk += rh - tl;
>+                  ip += (ab < wk) + wk;
>+                }
>+            }
>+        }
>+    }
>+}
>+
>+void
>+im (uint8_t kt)
>+{
>+  int wu = 0;
>+
>+  for (;;)
>+    {
>+      ++rm;
>+      if (rm < 1)
>+        {
>+          short int ms = 0;
>+
>+          while (kt < 1)
>+            {
>+              ms += rm < 0;
>+
>+              if (wu != 0)
>+                for (;;)
>+                  {
>+                  }
>+
>+              while (ms < 4)
>+                {
>+                  while (wu < 1)
>+                    wu += 2;
>+
>+                  ++ms;
>+                }
>+            }
>+
>+          if (ms == 0 || wu == 0)
>+            break;
>+        }
>+    }
>+
>+  while (wu < 1)
>+    while (qq < 1)
>+      ++qq;
>+}
>diff --git a/gcc/testsuite/gcc.c-torture/compile/pr86637-3.c
>b/gcc/testsuite/gcc.c-torture/compile/pr86637-3.c
>new file mode 100644
>index 0000000..6cb0fd7
>--- /dev/null
>+++ b/gcc/testsuite/gcc.c-torture/compile/pr86637-3.c
>@@ -0,0 +1,11 @@
>+/* { dg-options "-fsave-optimization-record -ftree-slp-vectorize
>--param ggc-min-expand=0 --param ggc-min-heapsize=1024" } */
>+void
>+te (void)
>+{
>+}
>+
>+int
>+main (void)
>+{
>+  return 0;
>+}
>diff --git a/gcc/testsuite/gcc.c-torture/compile/pr89235.c
>b/gcc/testsuite/gcc.c-torture/compile/pr89235.c
>new file mode 100644
>index 0000000..86be27f
>--- /dev/null
>+++ b/gcc/testsuite/gcc.c-torture/compile/pr89235.c
>@@ -0,0 +1,57 @@
>+/* { dg-require-effective-target fopenmp } */
>+/* { dg-options "-S -fopenmp -fsave-optimization-record
>-ftree-parallelize-loops=2 -fno-tree-vectorize --param
>ggc-min-expand=0" } */
>+
>+int a1, dr, xm, ly, zb, g9, il;
>+
>+long int wt;
>+unsigned int mq;
>+int br, e7, rm, t4, jb, ry;
>+
>+int
>+fi (void);
>+
>+int
>+z5 (int fl)
>+{
>+  while (br < 1)
>+    while (e7 != 0)
>+      while (mq != 1)
>+        {
>+          if (!!fl)
>+            {
>+              wt = rm;
>+              fi ();
>+            }
>+
>+          ++mq;
>+        }
>+
>+  return 0;
>+}
>+
>+void
>+gg (void)
>+{
>+  t4 = rm = z5 (rm);
>+  jb = z5 (rm);
>+  ry = fi ();
>+}
>+
>+#pragma omp declare simd
>+void
>+hl (void)
>+{
>+  for (;;)
>+    {
>+      gg ();
>+      gg ();
>+      gg ();
>+    }
>+}
>+
>+#pragma omp declare simd
>+int
>+cw (void)
>+{
>+  return 0;
>+}
>diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
>index eda4c24..d63d382 100644
>--- a/gcc/tree-vect-loop.c
>+++ b/gcc/tree-vect-loop.c
>@@ -8578,6 +8578,7 @@ optimize_mask_stores (struct loop *loop)
>   gimple_stmt_iterator gsi;
>   gimple *stmt;
>   auto_vec<gimple *> worklist;
>+  auto_purge_vect_location sentinel;
> 
>   vect_location = find_loop_location (loop);
>   /* Pick up all masked stores in loop if any.  */
>diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
>index fa5b22e..d271049 100644
>--- a/gcc/tree-vectorizer.c
>+++ b/gcc/tree-vectorizer.c
>@@ -86,6 +86,15 @@ along with GCC; see the file COPYING3.  If not see
> /* Loop or bb location, with hotness information.  */
> dump_user_location_t vect_location;
> 
>+/* auto_purge_vect_location's dtor: reset the vect_location
>+   global, to avoid stale location_t values that could reference
>+   GC-ed blocks.  */
>+
>+auto_purge_vect_location::~auto_purge_vect_location ()
>+{
>+  vect_location = dump_user_location_t ();
>+}
>+
> /* Dump a cost entry according to args to F.  */
> 
> void
>@@ -860,6 +869,7 @@ try_vectorize_loop_1 (hash_table<simduid_to_vf>
>*&simduid_to_vf_htab,
> {
>   unsigned ret = 0;
>   vec_info_shared shared;
>+  auto_purge_vect_location sentinel;
>   vect_location = find_loop_location (loop);
>if (LOCATION_LOCUS (vect_location.get_location_t ()) !=
>UNKNOWN_LOCATION
>       && dump_enabled_p ())
>@@ -1269,6 +1279,7 @@ public:
> unsigned int
> pass_slp_vectorize::execute (function *fun)
> {
>+  auto_purge_vect_location sentinel;
>   basic_block bb;
> 
>   bool in_loop_pipeline = scev_initialized_p ();
>@@ -1303,8 +1314,6 @@ pass_slp_vectorize::execute (function *fun)
>       loop_optimizer_finalize ();
>     }
> 
>-  vect_location = dump_user_location_t ();
>-
>   return 0;
> }
> 
>diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
>index d26b0f8..0e056f3 100644
>--- a/gcc/tree-vectorizer.h
>+++ b/gcc/tree-vectorizer.h
>@@ -1420,6 +1420,24 @@ extern dump_user_location_t vect_location;
> #define DUMP_VECT_SCOPE(MSG) \
>   AUTO_DUMP_SCOPE (MSG, vect_location)
> 
>+/* A sentinel class for ensuring that the "vect_location" global gets
>+   reset at the end of a scope.
>+
>+   The "vect_location" global is used during dumping and contains a
>+   location_t, which could contain references to a tree block via the
>+   ad-hoc data.  This data is used for tracking inlining information,
>+   but it's not a GC root; it's simply assumed that such locations
>never
>+   get accessed if the blocks are optimized away.
>+
>+   Hence we need to ensure that such locations are purged at the end
>+   of any operations using them (e.g. via this class).  */
>+
>+class auto_purge_vect_location
>+{
>+ public:
>+  ~auto_purge_vect_location ();
>+};
>+
> /*-----------------------------------------------------------------*/
> /* Function prototypes.                                            */
> /*-----------------------------------------------------------------*/