Re: [patch] Handle equivalent loop invariants as one in loop-invariant.c

[Ian Lance Taylor <ian at airs dot com>]

Zdenek Dvorak <rakdver@atrey.karlin.mff.cuni.cz> writes:

> > I don't really see how.  The only way that both Ei and MODEi could be
> > VOIDmode would be if a CONST_INT were compared to a CONST_INT.  And
> > the mode of a CONST_INT doesn't matter.  It is effectively the mode of
> > the parent.  And, as you say, the code has already checked that.
> 
> yes; and it is bad to rely on what someone might (or might not) do.

But it's not like we're talking about some random caller somewhere
else; we're talking about a recursive call from the function to
itself.

In the recursive call, the only way that submode1 will ever be
VOIDmode is if sub1 is a CONST_INT.  And in that case sub2 had better
also be CONST_INT, or the function will return false anyhow.  And so
the test winds up not telling you anything anyhow.  But that's not the
important point in any case....

> > There is code all over the compiler which does this sort of
> > equivalency checking, but doesn't bother to do the submode code.
> > Introducing it only here requires special justification--why does this
> > code require the extra attention?
> 
> huh?  We do this (keeping account of what mode the VOIDmode operand
> really is) all over the compiler; there is nothing special about this
> code.

....the function in question is quite similar to, e.g., rtx_equal_p,
or exp_equiv_p, or several others.  It's slightly different, so it
makes sense to have a different version.  But none of the other
versions worry about passing down the right mode of a CONST_INT.

Anything different catches the eye, and raises the potential for
confusion.  You have to justify why this function should be different
from the other functions that check for rtx equivalence under certain
conditions.

> Anyway, I don't really care.  I dislike the solution you propose,
> because its semantics is just weird; but not passing MODE1 and MODE2 to
> the function at all and requiring the caller to verify that the modes
> match is equivalent and somewhat cleaner (although a bit cumbersome to
> use).  I will prepare a patch with this change, if you insist.

I'm not going to approve the patch as it stands unless you can provide
some justification, in the form of a comment in the appropriate
places, for why this function should look different from other similar
functions.  (Obviously I'm not going to speak for other maintainers.)

If you want to tackle the problem in a different way, that is fine
with me.

Ian