Simplify SCEV entry

Message ID alpine.LSU.2.20.1710101231060.6597@zhemvz.fhfr.qr
State New
Headers show
Series
  • Simplify SCEV entry
Related show

Commit Message

Richard Biener Oct. 10, 2017, 10:34 a.m.
This simplifies SCEV analyze_scalar_evolution_1 by removing what appeared
to be dead code for the case of res != chrec_not_analyzed_yet.  Adding
a gcc_unrechable () turned up a rather serious bug though as in the
loop pipeline we get a messed up SCEV cache if CFG cleanup renumbers 
blocks given SCEV uses BB indices in the cache.  Fixing that makes
bootstrapping with gcc_unreachable in the place of the removed code
below work:

Comments

Andreas Schwab Oct. 11, 2017, 12:42 p.m. | #1
On Okt 10 2017, Richard Biener <rguenther@suse.de> wrote:

> 	* tree-cfgcleanup.c (cleanup_tree_cfg_noloop): Avoid compacting
> 	blocks if SCEV is active.
> 	* tree-scalar-evolution.c (analyze_scalar_evolution_1): Remove
> 	dead code.
> 	(analyze_scalar_evolution): Handle cached evolutions the obvious way.
> 	(scev_initialize): Assert we are not yet initialized.

That breaks gfortran.dg/graphite/pr68279.f90:

during GIMPLE pass: graphite
/daten/aranym/gcc/gcc-20171011/gcc/testsuite/gfortran.dg/graphite/pr68279.f90:8:0: internal compiler error: in set_codegen_error, at graphite-isl-ast-to-gimple.c:216
0x57056a translate_isl_ast_to_gimple::set_codegen_error()
	../../gcc/graphite-isl-ast-to-gimple.c:215
0x10eb522 translate_isl_ast_to_gimple::set_codegen_error()
	../../gcc/graphite-isl-ast-to-gimple.c:215
0x10eb522 translate_isl_ast_to_gimple::get_rename_from_scev(tree_node*, gimple**, loop*, basic_block_def*, basic_block_def*, vec<tree_node*, va_heap, vl_ptr>)
	../../gcc/graphite-isl-ast-to-gimple.c:1097
0x10ec77f translate_isl_ast_to_gimple::graphite_copy_stmts_from_block(basic_block_def*, basic_block_def*, vec<tree_node*, va_heap, vl_ptr>)
	../../gcc/graphite-isl-ast-to-gimple.c:1247
0x10ed9c9 translate_isl_ast_to_gimple::copy_bb_and_scalar_dependences(basic_block_def*, edge_def*, vec<tree_node*, va_heap, vl_ptr>)
	../../gcc/graphite-isl-ast-to-gimple.c:1313
0x10ee385 translate_isl_ast_to_gimple::translate_isl_ast_node_user(isl_ast_node*, edge_def*, std::map<isl_id*, tree_node*, std::less<isl_id*>, std::allocator<std::pair<isl_id* const, tree_node*> > >&)
	../../gcc/graphite-isl-ast-to-gimple.c:803
0x10ee74e translate_isl_ast_to_gimple::translate_isl_ast_for_loop(loop*, isl_ast_node*, edge_def*, tree_node*, tree_node*, tree_node*, std::map<isl_id*, tree_node*, std::less<isl_id*>, std::allocator<std::pair<isl_id* const, tree_node*> > >&)
	../../gcc/graphite-isl-ast-to-gimple.c:617
0x10ee9b3 translate_isl_ast_to_gimple::translate_isl_ast_node_for(loop*, isl_ast_node*, edge_def*, std::map<isl_id*, tree_node*, std::less<isl_id*>, std::allocator<std::pair<isl_id* const, tree_node*> > >&)
	../../gcc/graphite-isl-ast-to-gimple.c:721
0x10eeaa5 translate_isl_ast_to_gimple::translate_isl_ast_node_block(loop*, isl_ast_node*, edge_def*, std::map<isl_id*, tree_node*, std::less<isl_id*>, std::allocator<std::pair<isl_id* const, tree_node*> > >&)
	../../gcc/graphite-isl-ast-to-gimple.c:832
0x10eef9d graphite_regenerate_ast_isl(scop*)
	../../gcc/graphite-isl-ast-to-gimple.c:1559
0x10e9817 graphite_transform_loops()
	../../gcc/graphite.c:442
0x10ea7f0 graphite_transforms
	../../gcc/graphite.c:486
0x10ea7f0 execute
	../../gcc/graphite.c:563

Andreas.

Patch

Index: gcc/tree-scalar-evolution.c
===================================================================
--- gcc/tree-scalar-evolution.c (revision 253545)
+++ gcc/tree-scalar-evolution.c (working copy)
@@ -2072,8 +2072,11 @@  analyze_scalar_evolution_1 (struct loop
   if (res != chrec_not_analyzed_yet)
     {
       if (loop != bb->loop_father)
+       {
+         gcc_unreachable ();
        res = compute_scalar_evolution_in_loop
            (find_common_loop (loop, bb->loop_father), bb->loop_father, 
res);
+       }
 
       goto set_and_end;
     }

thus the following (which is really a bugfix plus code simplification).

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

I should probably backport that CFG cleanup hunk...

Richard.

2017-10-10  Richard Biener  <rguenther@suse.de>

	* tree-cfgcleanup.c (cleanup_tree_cfg_noloop): Avoid compacting
	blocks if SCEV is active.
	* tree-scalar-evolution.c (analyze_scalar_evolution_1): Remove
	dead code.
	(analyze_scalar_evolution): Handle cached evolutions the obvious way.
	(scev_initialize): Assert we are not yet initialized.

Index: gcc/tree-cfgcleanup.c
===================================================================
--- gcc/tree-cfgcleanup.c	(revision 253580)
+++ gcc/tree-cfgcleanup.c	(working copy)
@@ -892,7 +892,11 @@  cleanup_tree_cfg_noloop (void)
   changed |= cleanup_tree_cfg_1 ();
 
   gcc_assert (dom_info_available_p (CDI_DOMINATORS));
-  compact_blocks ();
+
+  /* Do not renumber blocks if the SCEV cache is active, it is indexed by
+     basic-block numbers.  */
+  if (! scev_initialized_p ())
+    compact_blocks ();
 
   checking_verify_flow_info ();
 
Index: gcc/tree-scalar-evolution.c
===================================================================
--- gcc/tree-scalar-evolution.c	(revision 253580)
+++ gcc/tree-scalar-evolution.c	(working copy)
@@ -281,7 +281,7 @@  along with GCC; see the file COPYING3.
 #include "tree-ssa-propagate.h"
 #include "gimple-fold.h"
 
-static tree analyze_scalar_evolution_1 (struct loop *, tree, tree);
+static tree analyze_scalar_evolution_1 (struct loop *, tree);
 static tree analyze_scalar_evolution_for_address_of (struct loop *loop,
 						     tree var);
 
@@ -2036,18 +2036,19 @@  compute_scalar_evolution_in_loop (struct
   if (no_evolution_in_loop_p (res, wrto_loop->num, &val) && val)
     return res;
 
-  return analyze_scalar_evolution_1 (wrto_loop, res, chrec_not_analyzed_yet);
+  return analyze_scalar_evolution_1 (wrto_loop, res);
 }
 
 /* Helper recursive function.  */
 
 static tree
-analyze_scalar_evolution_1 (struct loop *loop, tree var, tree res)
+analyze_scalar_evolution_1 (struct loop *loop, tree var)
 {
   tree type = TREE_TYPE (var);
   gimple *def;
   basic_block bb;
   struct loop *def_loop;
+  tree res;
 
   if (loop == NULL
       || TREE_CODE (type) == VECTOR_TYPE
@@ -2069,18 +2070,9 @@  analyze_scalar_evolution_1 (struct loop
       goto set_and_end;
     }
 
-  if (res != chrec_not_analyzed_yet)
-    {
-      if (loop != bb->loop_father)
-	res = compute_scalar_evolution_in_loop
-	    (find_common_loop (loop, bb->loop_father), bb->loop_father, res);
-
-      goto set_and_end;
-    }
-
   if (loop != def_loop)
     {
-      res = analyze_scalar_evolution_1 (def_loop, var, chrec_not_analyzed_yet);
+      res = analyze_scalar_evolution_1 (def_loop, var);
       res = compute_scalar_evolution_in_loop (loop, def_loop, res);
 
       goto set_and_end;
@@ -2144,7 +2136,8 @@  analyze_scalar_evolution (struct loop *l
     }
 
   res = get_scalar_evolution (block_before_loop (loop), var);
-  res = analyze_scalar_evolution_1 (loop, var, res);
+  if (res == chrec_not_analyzed_yet)
+    res = analyze_scalar_evolution_1 (loop, var);
 
   if (dump_file && (dump_flags & TDF_SCEV))
     fprintf (dump_file, ")\n");
@@ -3264,6 +3257,8 @@  scev_initialize (void)
 {
   struct loop *loop;
 
+  gcc_assert (! scev_initialized_p ());
+
   scalar_evolution_info = hash_table<scev_info_hasher>::create_ggc (100);
 
   initialize_scalar_evolutions_analyzer ();