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.
> 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.
Although TBH, the huge pushback that Kenny has got from this patch
puts me off ever trying that change.
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.
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.
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.
Storing a precision in wide_int in no way requires CONST_WIDE_INT
to store a mode. They are separate choices.
> The solution is not to have a CONST_WIDE_INT (again with VOIDmode
> and no precision in the RTX object(!)) and only have wide_int have a
> precision.
Why is having a VOIDmode CONST_WIDE_INT any worse than having
a VOIDmode CONST_INT or CONST_DOUBLE? In all three cases the mode
is being obtained/inferred from the same external source.
Richard
