On Fri, 21 Feb 2014, Richard Biener wrote: > On Fri, 21 Feb 2014, Richard Biener wrote: > > > > > This fixes the slowness of RTL expansion in PR60291 which is caused > > by excessive collisions in mem-attr sharing. The issue here is > > that sharing attempts happens across functions and we have a _lot_ > > of functions in this testcase referencing the same lexically > > equivalent memory, for example MEM[(StgWord *)_5 + -64B]. That > > means those get the same hash value. But they don't compare > > equal because an SSA name _5 from function A is of course not equal > > to one from function B. > > > > The following fixes that by not doing mem-attr sharing across functions > > by clearing the mem-attrs hashtable in rest_of_clean_state. > > > > Another fix may be to do what the comment in iterative_hash_expr > > says for SSA names: > > > > case SSA_NAME: > > /* We can just compare by pointer. */ > > return iterative_hash_host_wide_int (SSA_NAME_VERSION (t), val); > > > > (probably blame me for changing that to hashing the SSA version). > > It was lxo. > > > But I'm not sure that doesn't uncover issues with various hashtables and > > walking them, generating code dependent on the order. It's IMHO just not > > expected that you compare function-local expressions from different > > functions. > > Same speedup result from > > Index: gcc/tree.c > =================================================================== > --- gcc/tree.c (revision 207960) > +++ gcc/tree.c (working copy) > @@ -7428,7 +7428,7 @@ iterative_hash_expr (const_tree t, hashv > } > case SSA_NAME: > /* We can just compare by pointer. */ > - return iterative_hash_host_wide_int (SSA_NAME_VERSION (t), val); > + return iterative_hash_hashval_t ((uintptr_t)t>>3, val); > case PLACEHOLDER_EXPR: > /* The node itself doesn't matter. */ > return val; > > and from > > Index: gcc/tree.c > =================================================================== > --- gcc/tree.c (revision 207960) > +++ gcc/tree.c (working copy) > @@ -7428,7 +7428,9 @@ iterative_hash_expr (const_tree t, hashv > } > case SSA_NAME: > /* We can just compare by pointer. */ > - return iterative_hash_host_wide_int (SSA_NAME_VERSION (t), val); > + return iterative_hash_host_wide_int > + (DECL_UID (cfun->decl), > + iterative_hash_host_wide_int (SSA_NAME_VERSION (t), val)); > case PLACEHOLDER_EXPR: > /* The node itself doesn't matter. */ > return val; > > better than hashing pointers but requring cfun != NULL in this > function isn't good either. > > At this point I'm more comfortable with clearing the hashtable > than with changing iterative_hash_expr in any way. It's also > along the way to get rid of the hash completely. > > Oh, btw, the speedup is going from > > expand : 481.98 (94%) usr 1.15 (17%) sys 481.94 (93%) > wall 293891 kB (15%) ggc > > to > > expand : 2.66 ( 7%) usr 0.13 ( 2%) sys 2.64 ( 6%) > wall 262544 kB (13%) ggc > > at -O0 (less dramatic slowness for -On). > > > The other thing would be to discard mem-attr sharing alltogether, > > but that doesn't seem appropriate at this stage (but it would > > also simplify quite some code). With only one function in RTL > > at a time that shouldn't be too bad (see several suggestions > > along that line, even with statistics).
Last statistics: http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01784.html Richard.
