On Fri, May 3, 2013 at 2:37 PM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
Richard Biener <richard.guent...@gmail.com> writes:
See e.g. the hoops that cselib has to jump through:
/* We need to pass down the mode of constants through the hash table
functions. For that purpose, wrap them in a CONST of the appropriate
mode. */
static rtx
wrap_constant (enum machine_mode mode, rtx x)
{
if ((!CONST_SCALAR_INT_P (x)) && GET_CODE (x) != CONST_FIXED)
return x;
gcc_assert (mode != VOIDmode);
return gen_rtx_CONST (mode, x);
}
That is, cselib locally converts (const_int X) into (const:M (const_int X)),
purely so that it doesn't lose track of the CONST_INT's mode.
(const:M (const_int ...)) is invalid rtl elsewhere, but a necessary
hack here all the same.
Indeed ugly. But I wonder why cselib needs to store constants in
hashtables at all ... they should be VALUEs themselves. So the fix
for the above might not necessarily be to assign the CONST_INT
a mode (not that CONST_WIDE_INT would fix the above).
I don't understand. Do you mean that cselib values ought to have
a field to say whether the value is constant or not, and if so, what
constant that is? That feels like just the same kind of hack as the above.
The current idea of chaining all known equivalent rtxes in a list seems
more natural than having a list of all known equivalent rtxes except
CONST_INT and CONST_DOUBLE, which have to be stored separately instead.
(Again, we have runtime constants like SYMBOL_REF, which store modes,
and which would presumably still be in the normal rtx list.)
CONST_WIDE_INT was never supposed to solve this problem. I'm just giving
it as an example to back up the argument that rtx constants do in fact
have modes (although those modes are not stored in the rtx). The code
above is there to make sure that equivalence stays transitive.
Without it we could have bogus equivalences like:
(A) (reg:DI X) == (const_int Y) == (reg:SI Z)
even though it cannot be the case that:
(B) (reg:DI X) == (reg:SI Z)
My point is that, semantically, (A) did not come from X and Z being
equivalent to the "same" constant. X was equivalent to (const_int:DI Y)
and Z was equivalent to (const_int:SI Y). (A) only came about because
we happen to use the same rtx object to represent those two semantically-
distinct constants.
The idea isn't to make CONST_WIDE_INT get rid of the code above.
The idea is to make sure that wide_int has a precision and so doesn't
require code like the above to be written when dealing with wide_ints.
In other words, I got the impression your argument was "the fact
that CONST_INT and CONST_DOUBLE don't store a mode shows that
wide_int shouldn't store a precision". But the fact that CONST_INT
and CONST_DOUBLE don't store a mode doesn't mean they don't _have_
a mode. You just have to keep track of that mode separately.
And the same would apply to wide_int if we did the same thing there.
What I was trying to argue was that storing the mode/precision
separately is not always easy. It's also much less robust,
because getting the wrong mode or precision will only show up
for certain values. If the precision is stored in the wide_int,
mismatches can be asserted for based on precision alone, regardless
of the value.
I was just arguing that pointing out facts in the RTL land doesn't
necessarily influence wide-int which is purely separate. So if you
argue that having a mode in RTL constants would be soo nice and
thus that is why you want a precision in wide-int then I don't follow
that argument. If you want a mode in RTL constants then get a mode
in RTL constants!
This would make it immediately obvious where to get the precision
for wide-ints - something you do not address at all (and as you don't
I sort of cannot believe the 'it would be so nice to have a mode on RTL
constants').
That said, if modes on RTL constants were so useful then why not
have them on CONST_WIDE_INT at least? Please. Only sticking
them to wide-int in form of a precision is completely backward to me
(and I still think the core wide-int shouldn't have a precision, and if
you really want a wide-int-with-precision simply derive from wide-int).
Ok, so please then make all CONST_INTs and CONST_DOUBLEs have
a mode!
I'm saying that CONST_INT and CONST_DOUBLE already have a mode, but that
mode is not stored in the rtx. So if you're saying "make all CONST_INTs
and CONST_DOUBLEs _store_ a mode", then yeah, I'd like to :-) But I see
Kenny's patch as a prerequisite for that, because it consolidates the
CONST_INT and CONST_DOUBLE code so that the choice of rtx code is
less special. Lots more work is needed after that.
If there were a separate patch consolidating the paths I'd be all for
doing that.
I don't see a reason that this cannot be done even with the current
code using double-ints.
Although TBH, the huge pushback that Kenny has got from this patch
puts me off ever trying that change.
Well. The patch does so much together and is so large that makes
it basically unreviewable (or very hard to review at least).
But storing the mode in the rtx is orthogonal to what Kenny is doing.
The mode of each rtx constant is already available in the places
that Kenny is changing, because we already do the work to keep track
of the mode separately. Being able to get the mode directly from the
rtx would be simpler and IMO better, but the semantics are the same
either way.
Well, you showed examples where it is impossible to get at the mode.
Kenny's patch is not designed to "fix" the CONST_INT representation
(although the patch does make it easier to "fix" the representation
in future). Kenny's patch is about representing and handling constants
that we can't at the moment.
No, it is about much more.
The argument isn't whether CONST_WIDE_INT repeats "mistakes" made for
CONST_INT and CONST_DOUBLE; I hope we agree that CONST_WIDE_INT should
behave like the other two, whatever that is. The argument is about
whether we copy the "mistake" into the wide_int class.
I don't see how CONST_WIDE_INT is in any way related to wide_int other
than that you use wide_int to operate on the constants encoded in
CONST_WIDE_INT. As you have a mode available at the point you
create a wide_int from a CONST_WIDE_INT you can very easily just
use that modes precision to specify the precision of an operation
(or zero/sign-extend the result). That's what happens hidden in the
wide-int implementation currently, but in the awkward way that allows
precision mismatches and leads to odd things like having a wide-int
1 constant with a precision.
