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. > Storing a precision in wide_int in no way requires CONST_WIDE_INT > to store a mode. They are separate choices. Yes. And I obviously would have chosed to store a mode in CONST_WIDE_INT and no precision in wide_int. And I cannot see a good reason to do it the way you did it ;) >> 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. Well, we're arguing in circles - the argument that VOIDmode CONST_INT/DOUBLE are bad is yours. And if that's not bad I can't see why it is bad for wide-int to not have a mode (or precision). Richard. > Richard