Re: [tree-ssa] New regressions as of 2003-11-04

[Jan Hubicka <jh at suse dot cz>]

> 
> On Nov 4, 2003, at 3:51 PM, law@redhat.com wrote:
> 
> >In message <1067956749.23315.3.camel@frodo.toronto.redhat.com>, Diego 
> >Novillo w
> >rites:
> >>
> >>Lots of changes yesterday produced new regressions in C, C++, Fortran
> >>and mudflap.  I think I know what the mudflap problem is, so I'll take
> >>care of that.  Could you folks take a look at the new regressions and
> >>see if they're related to your changes?
> >>	 FAIL: gcc.dg/tree-ssa/20030814-4.c scan-tree-dump-times set = -1 0
> >>	 FAIL: gcc.dg/tree-ssa/20030814-5.c scan-tree-dump-times set = -1 0
> >OK.  After looking at these some more.  These are failures that are a
> >combination of the COND_EXPR lowering and some sillyness in PRE.
> >
> >Daniel -- did you actually bootstrap and regression test your PRE
> >changes before installing them?
> 
> Of course. I bootstrap every change i commit. If I hadn't tried to 
> bootstrap, i wouldn't have used the silly workaround to insert fake 
> assignments instead of empty statements (which somehow screw up the 
> statement linking and ends up with a cc1plus miscompilation, i'm still 
> working on tracking it down)
> I compared against the test results posted by diego for 
> i686-pc-linux-gnu, i must have missed these two in the noise.
> >
> >Another option would be for the out-of-ssa pass to handle it.  I can 
> >see
> >how it's a partitioning problem, but I don't immediately see how to
> >detect and optimizing this case in the out-of-ssa pass.
> >
> >Anyway, given that this is regression caused by the interaction between
> >Zdenek's lowering code and Daniel's edge splitting
> > code, one of them
> >really ought to be responsible for fixing it.
> 
> I'm not really sure how to fix it, and i'm trying to track down the 
> whole "inserting empty statements breaks cc1plus" problem, so i'd 
> appreciate if Zdenek could look at it.
> He understands this stuff better anyway.

I did just speak with Zdenek and he will look into it today.

On the related note, I implemented most of CFG transparent tree to RTL
expansion, but I am hitting problems of producing CFG failing
verify_flow_info because the CFG is already broken at expansion time.
I wrote basic checks to verify_flow_info.  I can not apply them right
now as they match way too often (split_edge is totally broken for
instance), but you may find it usefull when debugging problems.
(even when it does not know how to verify switch and abnormal edges yet)

Hope to have this resolved in a day or two so I can commit it.
So does it look OK assuming all problems on testsuite are fixed?

Wed Nov  5 10:04:56 CET 2003  Jan Hubicka  <jh@suse.cz>
	* tree-cfg.c (tree_block_label): Move; use create_artifical_label.
	(tree_verify_flow_info): Add code to check fallthru edges, GOTO and condjumps.
	* tree-optimize.c (optimize_function_tree): Check flow info.
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-cfg.c,v
retrieving revision 1.1.4.192
diff -c -3 -p -r1.1.4.192 tree-cfg.c
*** tree-cfg.c	3 Nov 2003 20:09:14 -0000	1.1.4.192
--- tree-cfg.c	5 Nov 2003 09:02:54 -0000
*************** tree_split_edge (edge edge_in)
*** 3406,3411 ****
--- 3410,3435 ----
    return new_bb;
  }
  
+ /* Return the label in the head of basic block BLOCK.  Create one if it doesn't
+    exist.  */
+ 
+ static tree
+ tree_block_label (basic_block bb)
+ {
+   tree stmt = first_stmt (bb);
+   /* We need a label at our final destination.  If it does not already exist,
+      create it.  */
+   if (!stmt || TREE_CODE (stmt) != LABEL_EXPR)
+     {
+       block_stmt_iterator iterator = bsi_start (bb);
+       tree label = create_artificial_label ();
+       stmt = build1 (LABEL_EXPR, void_type_node, label);
+       bsi_insert_before (&iterator, stmt, BSI_SAME_STMT);
+       return label;
+     }
+   else
+     return LABEL_EXPR_LABEL (stmt);
+ }
  
  /* Verifies that the flow information is OK.  */
  
*************** tree_verify_flow_info (void)
*** 3419,3437 ****
  
    FOR_EACH_BB (bb)
      {
        bsi = bsi_last (bb);
        if (bsi_end_p (bsi))
  	continue;
  
        stmt = bsi_stmt (bsi);
        switch (TREE_CODE (stmt))
  	{
  	case COND_EXPR:
! 	  if (TREE_CODE (COND_EXPR_THEN (stmt)) != GOTO_EXPR
! 	      || TREE_CODE (COND_EXPR_ELSE (stmt)) != GOTO_EXPR)
  	    {
! 	      fprintf (stderr, "Structured COND_EXPR at end of bb %d\n",
! 		       bb->index);
  	      err = 1;
  	    }
  	  break;
--- 3443,3509 ----
  
    FOR_EACH_BB (bb)
      {
+       edge e;
        bsi = bsi_last (bb);
        if (bsi_end_p (bsi))
  	continue;
  
+       for (e = bb->succ; e; e = e->succ_next)
+ 	if (e->flags & EDGE_FALLTHRU && e->dest != bb->next_bb)
+ 	  {
+ 	    error ("Fallthru edge of bb %d does not point to following block\n", bb->index);
+ 	    err = 1;
+ 	  }
+ 
+ 
        stmt = bsi_stmt (bsi);
        switch (TREE_CODE (stmt))
  	{
  	case COND_EXPR:
! 	  {
! 	    edge true_edge;
! 	    edge false_edge;
! 	    if (TREE_CODE (COND_EXPR_THEN (stmt)) != GOTO_EXPR
! 		|| TREE_CODE (COND_EXPR_ELSE (stmt)) != GOTO_EXPR)
! 	      {
! 		error ("Structured COND_EXPR at end of bb %d\n", bb->index);
! 		err = 1;
! 	      }
! 	    if (bb->succ->flags & EDGE_TRUE_VALUE)
! 	      true_edge = bb->succ, false_edge = bb->succ->succ_next;
! 	    else
! 	      false_edge = bb->succ, true_edge = bb->succ->succ_next;
! 	    if (!true_edge || !false_edge
! 		|| !(true_edge->flags & EDGE_TRUE_VALUE)
! 		|| !(false_edge->flags & EDGE_FALSE_VALUE)
! 		|| (true_edge->flags & (EDGE_FALLTHRU | EDGE_ABNORMAL))
! 		|| (true_edge->flags & (EDGE_FALLTHRU | EDGE_ABNORMAL))
! 		|| bb->succ->succ_next->succ_next)
! 	      {
! 		error ("Wrong outgoing edge flags at end of bb %d\n", bb->index);
! 		err = 1;
! 	      }
! 	    if (GOTO_DESTINATION (COND_EXPR_THEN (stmt))
! 		!= tree_block_label (true_edge->dest)
! 		|| (GOTO_DESTINATION (COND_EXPR_ELSE (stmt))
! 		    != tree_block_label (false_edge->dest)))
! 	      {
! 		error ("Label does not match edge at end of bb %d\n", bb->index);
! 		err = 1;
! 	      }
! 	  }
! 	  break;
! 	case GOTO_EXPR:
! 	  if (!bb->succ || bb->succ->succ_next
! 	      || (bb->succ->flags & (EDGE_FALLTHRU | EDGE_ABNORMAL
! 		  		     | EDGE_TRUE_VALUE | EDGE_FALSE_VALUE)))
  	    {
! 	      error ("Wrong outgoing edge flags at end of bb %d\n", bb->index);
! 	      err = 1;
! 	    }
! 	  if (GOTO_DESTINATION (stmt) != tree_block_label (bb->succ->dest))
! 	    {
! 	      error ("Label does not match edge at end of bb %d\n", bb->index);
  	      err = 1;
  	    }
  	  break;
*************** tree_make_forwarder_block (basic_block b
*** 3457,3463 ****
    basic_block dummy;
  
    /* Create the new basic block.  */
!   dummy = create_bb ();
    create_block_annotation (dummy);
    dummy->count = bb->count;
    dummy->frequency = bb->frequency;
--- 3529,3535 ----
    basic_block dummy;
  
    /* Create the new basic block.  */
!   dummy = create_bb (NULL);
    create_block_annotation (dummy);
    dummy->count = bb->count;
    dummy->frequency = bb->frequency;
*************** thread_jumps (void)
*** 3776,3805 ****
    return retval;
  }
  
- /* Return the label in the head of basic block BLOCK.  Create one if it doesn't
-    exist.  */
- 
- static tree
- tree_block_label (basic_block bb)
- {
-   tree stmt = first_stmt (bb);
-   /* We need a label at our final destination.  If it does not already exist,
-      create it.  */
-   if (!stmt || TREE_CODE (stmt) != LABEL_EXPR)
-     {
-       block_stmt_iterator iterator = bsi_start (bb);
-       tree label;
- 
-       label = build_decl (LABEL_DECL, NULL_TREE, NULL_TREE);
-       DECL_CONTEXT (label) = current_function_decl;
-       stmt = build1 (LABEL_EXPR, void_type_node, label);
-       bsi_insert_before (&iterator, stmt, BSI_SAME_STMT);
-       return label;
-     }
-   else
-     return LABEL_EXPR_LABEL (stmt);
- }
- 
  /* Attempt to perform edge redirection by replacing possibly complex jump
     instruction by goto or removing jump completely.  This can apply only
     if all edges now point to the same block.  The parameters and
--- 3848,3853 ----
Index: tree-optimize.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-optimize.c,v
retrieving revision 1.1.4.67
diff -c -3 -p -r1.1.4.67 tree-optimize.c
*** tree-optimize.c	3 Nov 2003 13:12:35 -0000	1.1.4.67
--- tree-optimize.c	5 Nov 2003 09:02:54 -0000
*************** optimize_function_tree (tree fndecl)
*** 80,89 ****
--- 80,95 ----
  
        /* Eliminate tail recursion calls.  */
        tree_optimize_tail_calls ();
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /* Compute aliasing information for all the variables referenced in
  	 the function.  */
        compute_may_aliases (fndecl);
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /*			BEGIN SSA PASSES
  
*************** optimize_function_tree (tree fndecl)
*** 96,105 ****
--- 102,117 ----
        /* Rewrite the function into SSA form.  Initially, request all
  	 variables to be renamed.  */
        rewrite_into_ssa (fndecl, NULL, TDI_ssa_1);
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /* Set up VARS_TO_RENAME to allow passes to inform which variables
  	 need to be renamed.  */
        vars_to_rename = sbitmap_alloc (num_referenced_vars);
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /* Perform dominator optimizations.  */
        if (flag_tree_dom)
*************** optimize_function_tree (tree fndecl)
*** 119,124 ****
--- 131,139 ----
  	 dead pointer assignments taking the address of local variables.  */
        if (flag_tree_dce)
  	tree_ssa_dce (fndecl, TDI_dce_1);
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /* The must-alias pass removes the aliasing and addressability bits
  	 from variables that used to have their address taken.  */
*************** optimize_function_tree (tree fndecl)
*** 131,136 ****
--- 146,154 ----
  	  if (sbitmap_first_set_bit (vars_to_rename) >= 0)
  	    rewrite_into_ssa (fndecl, vars_to_rename, TDI_ssa_3);
  	}
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /* Run SCCP (Sparse Conditional Constant Propagation).  */
        if (flag_tree_ccp)
*************** optimize_function_tree (tree fndecl)
*** 142,151 ****
--- 160,175 ----
  	  if (sbitmap_first_set_bit (vars_to_rename) >= 0)
  	    rewrite_into_ssa (fndecl, vars_to_rename, TDI_ssa_4);
  	}
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /* Run SSA-PRE (Partial Redundancy Elimination).  */
        if (flag_tree_pre)
  	tree_perform_ssapre (fndecl, TDI_pre);
+ #ifdef ENABLE_CHECKING
+       verify_flow_info ();
+ #endif
  
        /* Perform a second pass of dominator optimizations.  */
        if (flag_tree_dom)