diff mbox series

Fix PR85574

Message ID alpine.LSU.2.20.1901031315580.23386@zhemvz.fhfr.qr
State New
Headers show
Series Fix PR85574 | expand

Commit Message

Richard Biener Jan. 3, 2019, 12:20 p.m. UTC
The following rectifies a change during hash-map intruction which
changed the uncprop operand_equal_p based hash to a pointer-based
hash_map.  That assumes pointer equality between constants which
does not hold since different (but compatible) typed constants
can appear in the IL.  For SSA names the equivalence holds, of course.

Fixing this should increase the number of eliminated const-copies
on edges during out-of-SSA.  It also happens to fix the LTO
bootstrap miscompare of cc1 and friends, but I can't really
explain that.

[LTO] bootstrapped and tested on x86_64-unknown-linux-gnu, applied
to trunk.

Richard.

2019-01-03  Jan Hubicka  <hubicka@ucw.cz>

	PR tree-optimization/85574
	* tree-ssa-uncprop.c (struct equiv_hash_elt): Remove unused
	structure.
	(struct ssa_equip_hash_traits): Declare.
	(val_ssa_equiv): Use custom hash traits using operand_equal_p.

Comments

Richard Sandiford Jan. 3, 2019, 1:55 p.m. UTC | #1
Richard Biener <rguenther@suse.de> writes:
> The following rectifies a change during hash-map intruction which
> changed the uncprop operand_equal_p based hash to a pointer-based
> hash_map.  That assumes pointer equality between constants which
> does not hold since different (but compatible) typed constants
> can appear in the IL.  For SSA names the equivalence holds, of course.
>
> Fixing this should increase the number of eliminated const-copies
> on edges during out-of-SSA.  It also happens to fix the LTO
> bootstrap miscompare of cc1 and friends, but I can't really
> explain that.
>
> [LTO] bootstrapped and tested on x86_64-unknown-linux-gnu, applied
> to trunk.
>
> Richard.
>
> 2019-01-03  Jan Hubicka  <hubicka@ucw.cz>
>
> 	PR tree-optimization/85574
> 	* tree-ssa-uncprop.c (struct equiv_hash_elt): Remove unused
> 	structure.
> 	(struct ssa_equip_hash_traits): Declare.
> 	(val_ssa_equiv): Use custom hash traits using operand_equal_p.
>
> Index: gcc/tree-ssa-uncprop.c
> ===================================================================
> --- gcc/tree-ssa-uncprop.c	(revision 267549)
> +++ gcc/tree-ssa-uncprop.c	(working copy)
> @@ -268,21 +268,24 @@ associate_equivalences_with_edges (void)
>     so with each value we have a list of SSA_NAMEs that have the
>     same value.  */
>  
> -
> -/* Main structure for recording equivalences into our hash table.  */
> -struct equiv_hash_elt
> -{
> -  /* The value/key of this entry.  */
> -  tree value;
> -
> -  /* List of SSA_NAMEs which have the same value/key.  */
> -  vec<tree> equivalences;
> +/* Traits for the hash_map to record the value to SSA name equivalences
> +   mapping.  */
> +struct ssa_equip_hash_traits : default_hash_traits <tree>
> +{
> +  static inline hashval_t hash (value_type value)
> +    { return iterative_hash_expr (value, 0); }
> +  static inline bool equal (value_type existing, value_type candidate)
> +    { return operand_equal_p (existing, candidate, 0); }
>  };

FWIW, this is a dup of tree_operand_hash.

Thanks,
Richard

> +typedef hash_map<tree, auto_vec<tree>,
> +		 simple_hashmap_traits <ssa_equip_hash_traits,
> +					auto_vec <tree> > > val_ssa_equiv_t;
> +
>  /* Global hash table implementing a mapping from invariant values
>     to a list of SSA_NAMEs which have the same value.  We might be
>     able to reuse tree-vn for this code.  */
> -static hash_map<tree, auto_vec<tree> > *val_ssa_equiv;
> +val_ssa_equiv_t *val_ssa_equiv;
>  
>  static void uncprop_into_successor_phis (basic_block);
>  
> @@ -476,7 +479,7 @@ pass_uncprop::execute (function *fun)
>    associate_equivalences_with_edges ();
>  
>    /* Create our global data structures.  */
> -  val_ssa_equiv = new hash_map<tree, auto_vec<tree> > (1024);
> +  val_ssa_equiv = new val_ssa_equiv_t (1024);
>  
>    /* We're going to do a dominator walk, so ensure that we have
>       dominance information.  */
Richard Biener Jan. 3, 2019, 2:16 p.m. UTC | #2
On Thu, 3 Jan 2019, Richard Sandiford wrote:

> Richard Biener <rguenther@suse.de> writes:
> > The following rectifies a change during hash-map intruction which
> > changed the uncprop operand_equal_p based hash to a pointer-based
> > hash_map.  That assumes pointer equality between constants which
> > does not hold since different (but compatible) typed constants
> > can appear in the IL.  For SSA names the equivalence holds, of course.
> >
> > Fixing this should increase the number of eliminated const-copies
> > on edges during out-of-SSA.  It also happens to fix the LTO
> > bootstrap miscompare of cc1 and friends, but I can't really
> > explain that.
> >
> > [LTO] bootstrapped and tested on x86_64-unknown-linux-gnu, applied
> > to trunk.
> >
> > Richard.
> >
> > 2019-01-03  Jan Hubicka  <hubicka@ucw.cz>
> >
> > 	PR tree-optimization/85574
> > 	* tree-ssa-uncprop.c (struct equiv_hash_elt): Remove unused
> > 	structure.
> > 	(struct ssa_equip_hash_traits): Declare.
> > 	(val_ssa_equiv): Use custom hash traits using operand_equal_p.
> >
> > Index: gcc/tree-ssa-uncprop.c
> > ===================================================================
> > --- gcc/tree-ssa-uncprop.c	(revision 267549)
> > +++ gcc/tree-ssa-uncprop.c	(working copy)
> > @@ -268,21 +268,24 @@ associate_equivalences_with_edges (void)
> >     so with each value we have a list of SSA_NAMEs that have the
> >     same value.  */
> >  
> > -
> > -/* Main structure for recording equivalences into our hash table.  */
> > -struct equiv_hash_elt
> > -{
> > -  /* The value/key of this entry.  */
> > -  tree value;
> > -
> > -  /* List of SSA_NAMEs which have the same value/key.  */
> > -  vec<tree> equivalences;
> > +/* Traits for the hash_map to record the value to SSA name equivalences
> > +   mapping.  */
> > +struct ssa_equip_hash_traits : default_hash_traits <tree>
> > +{
> > +  static inline hashval_t hash (value_type value)
> > +    { return iterative_hash_expr (value, 0); }
> > +  static inline bool equal (value_type existing, value_type candidate)
> > +    { return operand_equal_p (existing, candidate, 0); }
> >  };
> 
> FWIW, this is a dup of tree_operand_hash.

Indeed.  Testing patch to do the obvious replacement.

Richard.

> Thanks,
> Richard
> 
> > +typedef hash_map<tree, auto_vec<tree>,
> > +		 simple_hashmap_traits <ssa_equip_hash_traits,
> > +					auto_vec <tree> > > val_ssa_equiv_t;
> > +
> >  /* Global hash table implementing a mapping from invariant values
> >     to a list of SSA_NAMEs which have the same value.  We might be
> >     able to reuse tree-vn for this code.  */
> > -static hash_map<tree, auto_vec<tree> > *val_ssa_equiv;
> > +val_ssa_equiv_t *val_ssa_equiv;
> >  
> >  static void uncprop_into_successor_phis (basic_block);
> >  
> > @@ -476,7 +479,7 @@ pass_uncprop::execute (function *fun)
> >    associate_equivalences_with_edges ();
> >  
> >    /* Create our global data structures.  */
> > -  val_ssa_equiv = new hash_map<tree, auto_vec<tree> > (1024);
> > +  val_ssa_equiv = new val_ssa_equiv_t (1024);
> >  
> >    /* We're going to do a dominator walk, so ensure that we have
> >       dominance information.  */
> 
>
Richard Biener Jan. 7, 2019, 2:32 p.m. UTC | #3
On Thu, 3 Jan 2019, Richard Biener wrote:

> On Thu, 3 Jan 2019, Richard Sandiford wrote:
> 
> > Richard Biener <rguenther@suse.de> writes:
> > > The following rectifies a change during hash-map intruction which
> > > changed the uncprop operand_equal_p based hash to a pointer-based
> > > hash_map.  That assumes pointer equality between constants which
> > > does not hold since different (but compatible) typed constants
> > > can appear in the IL.  For SSA names the equivalence holds, of course.
> > >
> > > Fixing this should increase the number of eliminated const-copies
> > > on edges during out-of-SSA.  It also happens to fix the LTO
> > > bootstrap miscompare of cc1 and friends, but I can't really
> > > explain that.
> > >
> > > [LTO] bootstrapped and tested on x86_64-unknown-linux-gnu, applied
> > > to trunk.
> > >
> > > Richard.
> > >
> > > 2019-01-03  Jan Hubicka  <hubicka@ucw.cz>
> > >
> > > 	PR tree-optimization/85574
> > > 	* tree-ssa-uncprop.c (struct equiv_hash_elt): Remove unused
> > > 	structure.
> > > 	(struct ssa_equip_hash_traits): Declare.
> > > 	(val_ssa_equiv): Use custom hash traits using operand_equal_p.
> > >
> > > Index: gcc/tree-ssa-uncprop.c
> > > ===================================================================
> > > --- gcc/tree-ssa-uncprop.c	(revision 267549)
> > > +++ gcc/tree-ssa-uncprop.c	(working copy)
> > > @@ -268,21 +268,24 @@ associate_equivalences_with_edges (void)
> > >     so with each value we have a list of SSA_NAMEs that have the
> > >     same value.  */
> > >  
> > > -
> > > -/* Main structure for recording equivalences into our hash table.  */
> > > -struct equiv_hash_elt
> > > -{
> > > -  /* The value/key of this entry.  */
> > > -  tree value;
> > > -
> > > -  /* List of SSA_NAMEs which have the same value/key.  */
> > > -  vec<tree> equivalences;
> > > +/* Traits for the hash_map to record the value to SSA name equivalences
> > > +   mapping.  */
> > > +struct ssa_equip_hash_traits : default_hash_traits <tree>
> > > +{
> > > +  static inline hashval_t hash (value_type value)
> > > +    { return iterative_hash_expr (value, 0); }
> > > +  static inline bool equal (value_type existing, value_type candidate)
> > > +    { return operand_equal_p (existing, candidate, 0); }
> > >  };
> > 
> > FWIW, this is a dup of tree_operand_hash.
> 
> Indeed.  Testing patch to do the obvious replacement.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Richard.

2019-01-07  Richard Biener  <rguenther@suse.de>

	* tree-ssa-uncprop.c (ssa_equip_hash_traits): Remove in favor
	of tree_operand_hash.

Index: gcc/tree-ssa-uncprop.c
===================================================================
--- gcc/tree-ssa-uncprop.c	(revision 267553)
+++ gcc/tree-ssa-uncprop.c	(working copy)
@@ -268,19 +268,7 @@ associate_equivalences_with_edges (void)
    so with each value we have a list of SSA_NAMEs that have the
    same value.  */
 
-/* Traits for the hash_map to record the value to SSA name equivalences
-   mapping.  */
-struct ssa_equip_hash_traits : default_hash_traits <tree>
-{
-  static inline hashval_t hash (value_type value)
-    { return iterative_hash_expr (value, 0); }
-  static inline bool equal (value_type existing, value_type candidate)
-    { return operand_equal_p (existing, candidate, 0); }
-};
-
-typedef hash_map<tree, auto_vec<tree>,
-		 simple_hashmap_traits <ssa_equip_hash_traits,
-					auto_vec <tree> > > val_ssa_equiv_t;
+typedef hash_map<tree_operand_hash, auto_vec<tree> > val_ssa_equiv_t;
 
 /* Global hash table implementing a mapping from invariant values
    to a list of SSA_NAMEs which have the same value.  We might be
diff mbox series

Patch

Index: gcc/tree-ssa-uncprop.c
===================================================================
--- gcc/tree-ssa-uncprop.c	(revision 267549)
+++ gcc/tree-ssa-uncprop.c	(working copy)
@@ -268,21 +268,24 @@  associate_equivalences_with_edges (void)
    so with each value we have a list of SSA_NAMEs that have the
    same value.  */
 
-
-/* Main structure for recording equivalences into our hash table.  */
-struct equiv_hash_elt
-{
-  /* The value/key of this entry.  */
-  tree value;
-
-  /* List of SSA_NAMEs which have the same value/key.  */
-  vec<tree> equivalences;
+/* Traits for the hash_map to record the value to SSA name equivalences
+   mapping.  */
+struct ssa_equip_hash_traits : default_hash_traits <tree>
+{
+  static inline hashval_t hash (value_type value)
+    { return iterative_hash_expr (value, 0); }
+  static inline bool equal (value_type existing, value_type candidate)
+    { return operand_equal_p (existing, candidate, 0); }
 };
 
+typedef hash_map<tree, auto_vec<tree>,
+		 simple_hashmap_traits <ssa_equip_hash_traits,
+					auto_vec <tree> > > val_ssa_equiv_t;
+
 /* Global hash table implementing a mapping from invariant values
    to a list of SSA_NAMEs which have the same value.  We might be
    able to reuse tree-vn for this code.  */
-static hash_map<tree, auto_vec<tree> > *val_ssa_equiv;
+val_ssa_equiv_t *val_ssa_equiv;
 
 static void uncprop_into_successor_phis (basic_block);
 
@@ -476,7 +479,7 @@  pass_uncprop::execute (function *fun)
   associate_equivalences_with_edges ();
 
   /* Create our global data structures.  */
-  val_ssa_equiv = new hash_map<tree, auto_vec<tree> > (1024);
+  val_ssa_equiv = new val_ssa_equiv_t (1024);
 
   /* We're going to do a dominator walk, so ensure that we have
      dominance information.  */