[PATCH] Fix PR42438

Richard Guenther rguenther@suse.de
Sun Jan 3 11:59:00 GMT 2010


This fixes some of the fallout of the fix of PR38819 by implementing
it in a more efficient and precise way.  PR38819 is about us
PREing trapping expressions where the path from the insertion point
to the redundant expression contains side-effects such as possibly
not returning calls.  The original fix simply avoided inserting
trapping expressions at all - which is overly conservative and
for example will never PRE integer divisions which is clearly
bad.  The following alternate fix instead tries to compute the
'contains side-effects on the path' property during antic set
cleaning thereby simulating noreturn edges from such calls.
(All non-const or looping calls are possibly not returning,
if we ever compute this property more precise we have to resort
to the side-effects notion again (consider __builtin_puts which
does return but clearly has side-effects)).

Simluating the extra edges in clean doesn't simulate the split
blocks, thus even calls after the original trapping expression
in the same basic-block will prohibit the PRE.  Still better
than before.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2010-01-03  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42438
	* tree-ssa-pre.c (struct bb_bitmap_sets): Add
	contains_may_not_return_call flag.
	(BB_MAY_NOTRETURN): New.
	(valid_in_sets): Trapping nary operations are not valid
	in blocks that may not return.
	(insert_into_preds_of_block): Remove check for trapping
	expressions.
	(compute_avail): Compute also BB_MAY_NOTRETURN.

	* gcc.dg/tree-ssa/ssa-pre-27.c: New testcase.

Index: gcc/tree-ssa-pre.c
===================================================================
*** gcc/tree-ssa-pre.c	(revision 155576)
--- gcc/tree-ssa-pre.c	(working copy)
*************** typedef struct bb_bitmap_sets
*** 382,392 ****
    bitmap expr_dies;
  
    /* True if we have visited this block during ANTIC calculation.  */
!   unsigned int visited:1;
  
    /* True we have deferred processing this block during ANTIC
       calculation until its successor is processed.  */
    unsigned int deferred : 1;
  } *bb_value_sets_t;
  
  #define EXP_GEN(BB)	((bb_value_sets_t) ((BB)->aux))->exp_gen
--- 382,395 ----
    bitmap expr_dies;
  
    /* True if we have visited this block during ANTIC calculation.  */
!   unsigned int visited : 1;
  
    /* True we have deferred processing this block during ANTIC
       calculation until its successor is processed.  */
    unsigned int deferred : 1;
+ 
+   /* True when the block contains a call that might not return.  */
+   unsigned int contains_may_not_return_call : 1;
  } *bb_value_sets_t;
  
  #define EXP_GEN(BB)	((bb_value_sets_t) ((BB)->aux))->exp_gen
*************** typedef struct bb_bitmap_sets
*** 399,404 ****
--- 402,408 ----
  #define EXPR_DIES(BB)	((bb_value_sets_t) ((BB)->aux))->expr_dies
  #define BB_VISITED(BB)	((bb_value_sets_t) ((BB)->aux))->visited
  #define BB_DEFERRED(BB) ((bb_value_sets_t) ((BB)->aux))->deferred
+ #define BB_MAY_NOTRETURN(BB) ((bb_value_sets_t) ((BB)->aux))->contains_may_not_return_call
  
  
  /* Basic block list in postorder.  */
*************** valid_in_sets (bitmap_set_t set1, bitmap
*** 2032,2037 ****
--- 2036,2048 ----
  		  return false;
  	      }
  	  }
+ 	/* If the NARY may trap make sure the block does not contain
+ 	   a possible exit point.
+ 	   ???  This is overly conservative if we translate AVAIL_OUT
+ 	   as the available expression might be after the exit point.  */
+ 	if (BB_MAY_NOTRETURN (block)
+ 	    && vn_nary_may_trap (nary))
+ 	  return false;
  	return true;
        }
        break;
*************** compute_antic (void)
*** 2469,2474 ****
--- 2480,2486 ----
  
        BB_VISITED (block) = 0;
        BB_DEFERRED (block) = 0;
+ 
        /* While we are here, give empty ANTIC_IN sets to each block.  */
        ANTIC_IN (block) = bitmap_set_new ();
        PA_IN (block) = bitmap_set_new ();
*************** insert_into_preds_of_block (basic_block 
*** 3187,3202 ****
  	}
      }
  
-   /* Make sure we are not inserting trapping expressions.  */
-   FOR_EACH_EDGE (pred, ei, block->preds)
-     {
-       bprime = pred->src;
-       eprime = avail[bprime->index];
-       if (eprime->kind == NARY
- 	  && vn_nary_may_trap (PRE_EXPR_NARY (eprime)))
- 	return false;
-     }
- 
    /* Make the necessary insertions.  */
    FOR_EACH_EDGE (pred, ei, block->preds)
      {
--- 3199,3204 ----
*************** compute_avail (void)
*** 3804,3809 ****
--- 3806,3813 ----
        for (gsi = gsi_start_phis (block); !gsi_end_p (gsi); gsi_next (&gsi))
  	make_values_for_phi (gsi_stmt (gsi), block);
  
+       BB_MAY_NOTRETURN (block) = 0;
+ 
        /* Now compute value numbers and populate value sets with all
  	 the expressions computed in BLOCK.  */
        for (gsi = gsi_start_bb (block); !gsi_end_p (gsi); gsi_next (&gsi))
*************** compute_avail (void)
*** 3814,3819 ****
--- 3818,3840 ----
  	  stmt = gsi_stmt (gsi);
  	  gimple_set_uid (stmt, stmt_uid++);
  
+ 	  /* Cache whether the basic-block has any non-visible side-effect
+ 	     or control flow.
+ 	     If this isn't a call or it is the last stmt in the
+ 	     basic-block then the CFG represents things correctly.  */
+ 	  if (is_gimple_call (stmt)
+ 	      && !stmt_ends_bb_p (stmt))
+ 	    {
+ 	      /* Non-looping const functions always return normally.
+ 		 Otherwise the call might not return or have side-effects
+ 		 that forbids hoisting possibly trapping expressions
+ 		 before it.  */
+ 	      int flags = gimple_call_flags (stmt);
+ 	      if (!(flags & ECF_CONST)
+ 		  || (flags & ECF_LOOPING_CONST_OR_PURE))
+ 		BB_MAY_NOTRETURN (block) = 1;
+ 	    }
+ 
  	  FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_DEF)
  	    {
  	      pre_expr e = get_or_alloc_expr_for_name (op);
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-27.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-27.c	(revision 0)
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-27.c	(revision 0)
***************
*** 0 ****
--- 1,29 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-tree-pre" } */
+ 
+ int foo (int i, int j, int b)
+ {
+   int res = 0;
+   if (b)
+     res = i/j;
+   /* We should insert the possibly trapping i/j.  */
+   res += i/j;
+   return res;
+ }
+ 
+ extern void bar (void);
+ int foo2 (int i, int j, int b)
+ {
+   int res = 0;
+   if (b)
+     res = i/j;
+   /* But we fail so here because of the possibly not returning
+      call in the same basic-block.  */
+   res += i/j;
+   bar ();
+   return res;
+ }
+ 
+ /* { dg-final { scan-tree-dump-times "# prephitmp" 1 "pre" } } */
+ /* { dg-final { scan-tree-dump-times "# prephitmp" 2 "pre" { xfail *-*-* } } } */
+ /* { dg-final { cleanup-tree-dump "pre" } } */



More information about the Gcc-patches mailing list