From 25b67546a1a5fd49ddcb0a923a8d23d77a91e05f Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 7 Feb 2019 23:00:18 +0000 Subject: [PATCH] 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. 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. From-SVN: r268659 --- gcc/ChangeLog | 15 ++ gcc/testsuite/ChangeLog | 9 ++ .../gcc.c-torture/compile/pr86637-1.c | 13 ++ .../gcc.c-torture/compile/pr86637-2.c | 128 ++++++++++++++++++ .../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 +++ 9 files changed, 263 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/ChangeLog b/gcc/ChangeLog index 098479015d4..cbb1ebe4a3e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,18 @@ +2019-02-07 David Malcolm + + 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. + 2019-02-07 Kyrylo Tkachov * config/aarch64/iterators.md (max_opp): New code_attr. diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 9951b17cd18..cb500f53489 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2019-02-07 David Malcolm + + 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. + 2019-02-07 Kyrylo Tkachov * gcc.target/aarch64/abd_1.c: New test. 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 00000000000..61b6381b224 --- /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 00000000000..3b675eae1b6 --- /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 +#include + +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 00000000000..6cb0fd78e97 --- /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 00000000000..86be27f7704 --- /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 eda4c24846a..d63d3825f20 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 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 fa5b22efeba..d27104933a9 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_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 d26b0f83bd1..0e056f34b53 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. */ /*-----------------------------------------------------------------*/ -- 2.30.2