Richard Guenther <richard.guent...@gmail.com> writes: > On Fri, Oct 5, 2012 at 1:24 PM, Richard Sandiford > <rdsandif...@googlemail.com> wrote: >> Richard Guenther <richard.guent...@gmail.com> writes: >>> The issue is that unlike RTL where we "construct" double-ints from >>> CONST_INT/CONST_DOUBLE right now, tree has the double-int >>> _embedded_. That's why I say that the thing we embed in >>> a CONST_WIDE_INT or a tree INTEGER_CST needs to be >>> "bare metal", and that's what I would call wide-int. >> >> OK, but that's deliberately not what Kenny's patch calls "wide int". >> The whole idea is that the "bare metal" thing we embed in a >> CONST_WIDE_INT or tree isn't (and doesn't need to be) the same >> as the type that we use to operate on integers. That bare-metal >> thing doesn't even have to have a fixed width. (CONST_WIDE_INT >> doesn't have a fixed width, it's only as big as the integer >> needs it to be.) > > Ok, let's rephrase it this way then: the "bare metal" thing used > for the storage should ideally be the same in the tree IL and the RTL > IL _and_ the higher-level abstract wide-int.
Hmm, OK, that's a straight disagreement then. >>> So to me wide-ints provide the higher-level abstraction ontop of >>> double-ints (which would remain the storage entity). >>> >>> Such higher-level abstraction is very useful, also for double-ints and >>> also on the tree level. There is no requirement to provide bigger >>> double-int (or wide-int) for this. Let's call this abstraction >>> wide-int (as opposed to my suggested more piecemail double_sint, >>> double_uint). You can perfectly model it ontop of the existing >>> double_int storage. >>> >>> As of providing larger "double-ints" - there is not much code left >>> (ok, quite an overstatement ;)) that relies on the implementation >>> detail of double-int containing exactly two HOST_WIDE_INTs. >>> The exact purpose of double-ints was to _hide_ this (previously >>> we only had routines like mul_double_with_sign which take >>> two HOST_WIDE_INT components). Most code dealing with >>> the implementation detail is code interfacing with middle-end >>> routines that take a HOST_WIDE_INT argument (thus the >>> double_int_fits_hwi_p predicates) - even wide_int has to support >>> this kind of interfacing. >>> >>> So, after introducing wide_int that just wraps double_int and >>> changing all user code (hopefully all, I guess mostly all), we'd >>> tackle the issue that the size of double_int's is host dependent. >>> A simple solution is to make it's size dependent on a target macro >>> (yes, macro ...), so on a 32bit HWI host targeting a 64bit 'HWI' target >>> you'd simply have four HWIs in the 'double-int' storage (and >>> arithmetic needs to be generalized to support this). >> >> I just don't see why this is a good thing. The constraints >> are different when storing integers and when operating on them. >> When operating on them, we want something that is easily constructed >> on the stack, so we can create temporary structures very cheaply, >> and can easily pass and return them. We happen to know at GCC's >> compile time how big the biggest integer will ever be, so it makes >> sense for wide_int to be that wide. > > I'm not arguing against this. I'm just saying that the internal > representation will depend on the host - not the number of total > bits, but the number of pieces. Sure, but Kenny already has a macro to specify how many bits we need (MAX_BITSIZE_MODE_ANY_INT). We can certainly wrap: HOST_WIDE_INT val[MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT]; in a typedef if that's what you prefer. >> But when storing integers we want something compact. If your >> machine supports 256-bit integers, but the code you're compiling >> makes heavy use of 128-bit integers, why would you want to waste >> 128 of 256 bits on every stored integer? Which is why even >> CONST_WIDE_INT doesn't have a fixed width. >> >> You seem to be arguing that the type used to store integers in the IR >> has to be same as the one that we use when performing compile-time >> arithmetic, but I think it's better to have an abstraction between >> the two. > > Well, you don't want to pay the cost dividing 256bit numbers all > the time when most of your numbers are only 128bit. So we don't > really want to perform compile-time arithmetic on the biggest > possible precision either. wide_int doesn't perform them in the biggest possible precision. It performs them in the precision of the result. It just happens to have enough storage to cope with all possible precisions (because the actual precision of the result is usually only known at GCC's runtime). >> So if you think a pair of HWIs continues to be a useful way of >> storing integers at the tree level, then we can easily continue >> to use a pair of HWIs there. > > How do you store larger ints there then? You'd need another tree code for wider integers. I'm not saying that's a good idea, I just wasn't sure if it's what you wanted. > How is CONST_WIDE_INT variable size? It's just the usual trailing variable-length array thing. > Why can wide-int not be variable-size? Because variable-length arrays are usually more expensive than (still fairly small) fixed-length arrays when dealing with temporaries. >> Or if you'd prefer every tree integer >> to be the same width as a wide_int, we can do that too. (I don't >> know what Kenny's tree patch does.) > > Keenys patch truncates wide-ints to two HWIs in wide-int-to-tree > without any checking (I claimed it's a bug, Kenny says its a feature). Only because he split the rtl and tree parts up. By "Kenny's tree patch", I meant the patch that he said he was going to send (part 4 as he called it). Until then, we're not regressing at the tree level, and I think the patch is a genuine RTL improvement on its own. >> Another advantage of abstracting away the storage type is that >> we could store things like an overflow flag in the wide_int >> (if that ever proves useful) without worrying about the impact >> on the tree and RTL footprint. > > Sure. > > We seem to be talking in circles. You don't seem to want (or care) > about a common storage for tree, RTL and wide-int. You seem to > care about that operate-on-wide-ints thing. I am saying if you keep > double-ints as-is you create two similar things which should be one thing. The idea is to get rid of double_ints altogether. They shouldn't have any use once everything has been converted to wide_ints. > So, make double-int storage only. The idea was to treat them as legacy and get rid of them as soon as we can. > What I don't see is that the patch just introduces wide-int as type > to do compile-time math on in RTL. It does more (the patch is large > and I didn't read it in detail). Yeah, it introduces things like the CONST_SCALAR_INT_P abstraction. But I actually find the patch easier to review like that, because both changes are affecting the same kinds of place. > It doesn't even try to address the same on the tree level. Because as Kenny's already said, that's a separate patch. > It doesn't address the fundamental issue of double-int size being host > dependent. Because the plan is to get rid of it :-) * Trivial, but it has the wrong name. * It has the wrong interface for general-precision arithmetic because it doesn't say how wide the value stored (or to be stored) in those HOST_WIDE_INTs actually is. E.g. there's no such thing as an "X-bit add followed by an X-bit division". You have to a "double_int-wide add", followed by a truncation/extension to X bits, followed by a "double_int-wide division", followed by another truncation/extension to X bits. Which we don't do in RTL; we just assume or (occasionally) assert that we only use double_int for modes whose precisions are exactly 2 * HOST_BITS_PER_WIDE_INT. * Using a fixed-length array of HOST_WIDE_INTs is too inflexible for size-conscious IRs, so just bumping its size probably isn't a good idea for them. But having a fixed-length array does IMO make sense for temporaries. * If we made it storage-only, it doesn't need all those operations. Which is why I agree with Kenny that double_int as it exists today isn't the right starting point. Richard
