Message ID | alpine.LSU.2.20.1901031315580.23386@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
Series | Fix PR85574 | expand |
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. */
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. */ > >
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
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. */