On February 7, 2019 9:10:15 PM GMT+01:00, David Malcolm <dmalc...@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 = μ
>+
>+ 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. */
> /*-----------------------------------------------------------------*/
