diff mbox

Fix PR56521

Message ID alpine.LNX.2.00.1303051257070.3543@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener March 5, 2013, 11:57 a.m. UTC 
On Tue, 5 Mar 2013, Jakub Jelinek wrote:

> On Tue, Mar 05, 2013 at 12:51:09PM +0100, Richard Biener wrote:
> > VN now inserts all sorts of calls into the references hashtable,
> > not only those which produce a value.  This results in missing
> > initializations of ->value_id which eventually PRE ends up
> > accessing.
> > 
> > The following fixes that.
> > 
> > Bootstrap and regtest pending on x86_64-unknown-linux-gnu.
> > 
> > Richard.
> > 
> > 2013-03-05  Richard Biener  <rguenther@suse.de>
> > 
> > 	* tree-ssa-sccvn.c (set_value_id_for_result): For a NULL
> > 	result set a new value-id.
> > 
> > --- gcc/tree-ssa-sccvn.c	(revision 196451)
> > +++ gcc/tree-ssa-sccvn.c	(working copy)
> > @@ -3954,7 +3962,7 @@ free_scc_vn (void)
> >    XDELETE (optimistic_info);
> >  }
> >  
> > -/* Set *ID if we computed something useful in RESULT.  */
> > +/* Set *ID according to RESULT.  */
> >  
> >  static void
> >  set_value_id_for_result (tree result, unsigned int *id)
> > @@ -3966,6 +3974,8 @@ set_value_id_for_result (tree result, un
> >        else if (is_gimple_min_invariant (result))
> >  	*id = get_or_alloc_constant_value_id (result);
> 
> This still won't initialize *id if result is non-NULL, but isn't
> SSA_NAME nor is_gimple_min_invariant.  Can't you do the same for that case
> too, just in case (perhaps we can't trigger that right now, but still
> it would make me feel safer about that).

Yeah.  Can happen from aggregate stores I gues.

> >      }
> > +  else
> > +    *id = get_next_value_id ();
> >  }
> >  
> >  /* Set the value ids in the valid hash tables.  */
> 
> Otherwise looks good to me, thanks.

As follows.

Richard.

2013-03-05  Richard Biener  <rguenther@suse.de>

	* tree-ssa-sccvn.c (set_value_id_for_result): For a NULL
	result set a new value-id.

Comments

Jakub Jelinek March 5, 2013, 12:03 p.m. UTC | #1 
On Tue, Mar 05, 2013 at 12:57:41PM +0100, Richard Biener wrote:
> As follows.
> 
> Richard.
> 
> 2013-03-05  Richard Biener  <rguenther@suse.de>
> 
> 	* tree-ssa-sccvn.c (set_value_id_for_result): For a NULL
> 	result set a new value-id.

Looks much better.
You forgot to adjust the ChangeLog entry, and PR line is missing,
if it passes bootstrap, please check it in.

> --- gcc/tree-ssa-sccvn.c	(revision 196451)
> +++ gcc/tree-ssa-sccvn.c	(working copy)
> @@ -3954,18 +3962,17 @@ free_scc_vn (void)
>    XDELETE (optimistic_info);
>  }
>  
> -/* Set *ID if we computed something useful in RESULT.  */
> +/* Set *ID according to RESULT.  */
>  
>  static void
>  set_value_id_for_result (tree result, unsigned int *id)
>  {
> -  if (result)
> -    {
> -      if (TREE_CODE (result) == SSA_NAME)
> -	*id = VN_INFO (result)->value_id;
> -      else if (is_gimple_min_invariant (result))
> -	*id = get_or_alloc_constant_value_id (result);
> -    }
> +  if (result && TREE_CODE (result) == SSA_NAME)
> +    *id = VN_INFO (result)->value_id;
> +  else if (result && is_gimple_min_invariant (result))
> +    *id = get_or_alloc_constant_value_id (result);
> +  else
> +    *id = get_next_value_id ();
>  }
>  
>  /* Set the value ids in the valid hash tables.  */

	Jakub
diff mbox

Patch

Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c	(revision 196451)
+++ gcc/tree-ssa-sccvn.c	(working copy)
@@ -3954,18 +3962,17 @@  free_scc_vn (void)
   XDELETE (optimistic_info);
 }
 
-/* Set *ID if we computed something useful in RESULT.  */
+/* Set *ID according to RESULT.  */
 
 static void
 set_value_id_for_result (tree result, unsigned int *id)
 {
-  if (result)
-    {
-      if (TREE_CODE (result) == SSA_NAME)
-	*id = VN_INFO (result)->value_id;
-      else if (is_gimple_min_invariant (result))
-	*id = get_or_alloc_constant_value_id (result);
-    }
+  if (result && TREE_CODE (result) == SSA_NAME)
+    *id = VN_INFO (result)->value_id;
+  else if (result && is_gimple_min_invariant (result))
+    *id = get_or_alloc_constant_value_id (result);
+  else
+    *id = get_next_value_id ();
 }
 
 /* Set the value ids in the valid hash tables.  */