Alexandre Oliva <aol...@redhat.com> writes:
> Jakub asked to have a closer look at the problem, and I found we could
> do somewhat better. The first thing I noticed was that the problem was
> that, in each block that computed a (base+const), we created a new VALUE
> for the expression (with the same const and global base), and a new
> reverse operation.
>
> This was wrong. Clearly we should reuse the same expression. I had to
> arrange for the expression to be retained across basic blocks, for it
> was function invariant. I split out the code to detect invariants from
> the function that removes entries from the cselib hash table across
> blocks, and made it recursive so that a VALUE equivalent to (plus
> (value) (const_int)) will be retained, if the base value fits (maybe
> recursively) the definition of invariant.
>
> An earlier attempt to address this issue remained in cselib: using the
> canonical value to build the reverse expression. I believe it has a
> potential of avoiding the creation of redundant reverse expressions, for
> expressions involving equivalent but different VALUEs will evaluate to
> different hashes. I haven't observed effects WRT the given testcase,
> before or after the change that actually fixed the problem, because we
> now find the same base expression and thus reuse the reverse_op as well,
> but I figured I'd keep it in for it is very cheap and possibly useful.
Thanks for looking at this. Just to be sure: does this avoid the kind
of memrefs_conflict_p cycle I was seeing in:
http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01051.html
(in theory, I mean).
Richard
