Patchwork Fix VRP handling of undefined state

login
register
mail settings
Submitter Richard Guenther
Date July 29, 2011, 8:46 a.m.
Message ID <alpine.LNX.2.00.1107291044391.810@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/107358/
State New
Headers show

Comments

Richard Guenther - July 29, 2011, 8:46 a.m.
I noticed that for binary expressions VRP contains the same bugs
that CCP once did (it treats UNDEFINED * 0 as UNDEFINED).  Then
I noticed we never hit this bug because we never end up with
any range being UNDEFINED - which is bad, because this way we miss
value-ranges for all conditionally initialized variables.

Thus, the following patch fixes this and conservatively handles
the binary expression case (which is only of academic interest
anyway - the important part is to handle UNDEFINED in PHIs).

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

Richard.

2011-07-29  Richard Guenther  <rguenther@suse.de>

	* tree-vrp.c (get_value_range): Only set parameter default
	definitions to varying, leave others at undefined.
	(extract_range_from_binary_expr): Fix undefined handling.
	(vrp_visit_phi_node): Handle merged undefined state.

	* gcc.dg/uninit-suppress.c: Also disable VRP.
	* gcc.dg/uninit-suppress_2.c: Likewise.


Index: gcc/testsuite/gcc.dg/uninit-suppress.c
===================================================================
*** gcc/testsuite/gcc.dg/uninit-suppress.c	(revision 176869)
--- gcc/testsuite/gcc.dg/uninit-suppress.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-fno-tree-ccp -O2 -Wuninitialized -Wno-maybe-uninitialized" } */
  void blah();
  int gflag;
  
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-fno-tree-ccp -fno-tree-vrp -O2 -Wuninitialized -Wno-maybe-uninitialized" } */
  void blah();
  int gflag;
  
Index: gcc/testsuite/gcc.dg/uninit-suppress_2.c
===================================================================
*** gcc/testsuite/gcc.dg/uninit-suppress_2.c	(revision 176869)
--- gcc/testsuite/gcc.dg/uninit-suppress_2.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-fno-tree-ccp -O2 -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */
  void blah();
  int gflag;
  
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-fno-tree-ccp -fno-tree-vrp -O2 -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */
  void blah();
  int gflag;
H.J. Lu - May 24, 2012, 2:50 a.m.
On Fri, Jul 29, 2011 at 1:46 AM, Richard Guenther <rguenther@suse.de> wrote:
>
> I noticed that for binary expressions VRP contains the same bugs
> that CCP once did (it treats UNDEFINED * 0 as UNDEFINED).  Then
> I noticed we never hit this bug because we never end up with
> any range being UNDEFINED - which is bad, because this way we miss
> value-ranges for all conditionally initialized variables.
>
> Thus, the following patch fixes this and conservatively handles
> the binary expression case (which is only of academic interest
> anyway - the important part is to handle UNDEFINED in PHIs).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>
> Richard.
>
> 2011-07-29  Richard Guenther  <rguenther@suse.de>
>
>        * tree-vrp.c (get_value_range): Only set parameter default
>        definitions to varying, leave others at undefined.
>        (extract_range_from_binary_expr): Fix undefined handling.
>        (vrp_visit_phi_node): Handle merged undefined state.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

Patch

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 176869)
+++ gcc/tree-vrp.c	(working copy)
@@ -692,16 +692,16 @@  get_value_range (const_tree var)
   /* Defer allocating the equivalence set.  */
   vr->equiv = NULL;
 
-  /* If VAR is a default definition, the variable can take any value
-     in VAR's type.  */
+  /* If VAR is a default definition of a parameter, the variable can
+     take any value in VAR's type.  */
   sym = SSA_NAME_VAR (var);
-  if (SSA_NAME_IS_DEFAULT_DEF (var))
+  if (SSA_NAME_IS_DEFAULT_DEF (var)
+      && TREE_CODE (sym) == PARM_DECL)
     {
       /* Try to use the "nonnull" attribute to create ~[0, 0]
 	 anti-ranges for pointers.  Note that this is only valid with
 	 default definitions of PARM_DECLs.  */
-      if (TREE_CODE (sym) == PARM_DECL
-	  && POINTER_TYPE_P (TREE_TYPE (sym))
+      if (POINTER_TYPE_P (TREE_TYPE (sym))
 	  && nonnull_arg_p (sym))
 	set_value_range_to_nonnull (vr, TREE_TYPE (sym));
       else
@@ -2225,12 +2225,20 @@  extract_range_from_binary_expr (value_ra
   else
     set_value_range_to_varying (&vr1);
 
-  /* If either range is UNDEFINED, so is the result.  */
-  if (vr0.type == VR_UNDEFINED || vr1.type == VR_UNDEFINED)
+  /* If both ranges are UNDEFINED, so is the result.  */
+  if (vr0.type == VR_UNDEFINED && vr1.type == VR_UNDEFINED)
     {
       set_value_range_to_undefined (vr);
       return;
     }
+  /* If one of the ranges is UNDEFINED drop it to VARYING for the following
+     code.  At some point we may want to special-case operations that
+     have UNDEFINED result for all or some value-ranges of the not UNDEFINED
+     operand.  */
+  else if (vr0.type == VR_UNDEFINED)
+    set_value_range_to_varying (&vr0);
+  else if (vr1.type == VR_UNDEFINED)
+    set_value_range_to_varying (&vr1);
 
   /* The type of the resulting value range defaults to VR0.TYPE.  */
   type = vr0.type;
@@ -6642,6 +6650,8 @@  vrp_visit_phi_node (gimple phi)
 
   if (vr_result.type == VR_VARYING)
     goto varying;
+  else if (vr_result.type == VR_UNDEFINED)
+    goto update_range;
 
   old_edges = vr_phi_edge_counts[SSA_NAME_VERSION (lhs)];
   vr_phi_edge_counts[SSA_NAME_VERSION (lhs)] = edges;
@@ -6713,6 +6723,7 @@  vrp_visit_phi_node (gimple phi)
 
   /* If the new range is different than the previous value, keep
      iterating.  */
+update_range:
   if (update_value_range (lhs, &vr_result))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))