This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

patch to add storage classes to wide int.


Richi

this patch is our attempt to implement the storage classes for wide int as you suggested. The patch currently does not work for reasons that will be mentioned below, but we stopped work on it because it is clear that this is a terrible idea.

1) Having more than one storage class is like having more than one register in an architecture: 1 register register allocation is easy, more than one register is np complete. More than one storage class makes the code unreasonably complex. While it is true that in many cases, this can be "hidden" with c++ magic, in a lot of places it cannot and the notion that all of this is free is just a myth.

The storage classes mechanism that you have suggested, is not polymorphic. It is two separate classes for wide it, one that does pointer copying and one that stack allocates. From the client's perspective, this does not appear too bad, just a lot of <> here and there.

But there are a lot of places where, at run time, it is not clear which of the two alternatives is going to be available. Consider the following code:

wide_int<>::foo = wide_int<ptr>::from_rtx (x);

if (...)
   foo = foo + wide_int<ptr>::from_rtx (y);

Which type of wide-int does foo contain after the if-then. It really could be either one and so one of the two has to be converted into the other one before the next use of foo. I claimed, i believe correctly that data copying was not going to be any more expensive than pointer copying because the amount of data was so small. But now, because we have more than one type, we have to deal with conversion.

2) The patch does not work for rtxes at all. Rtxes have to copied. Trees could be pointer copied.
The problem is that CONST_INTs are not canonized in a way that wide-ints are or that trees could be.
This comes from the use of the GEN_INT macro that does not take a mode. Without a mode, you do not get the integers into proper form: i.e. sign extended. At the tree level, this is not a problem because the INT_CST constructors take a type. But the best that can be done at the rtl level is to do the sign extension when we convert inside from_rtx (which takes a precision or a mode).


Fixing this is on Richard Sandiford's and my "it would be nice list" but is certainly out of the question to do as a prereq for this patch.

3) The patch seems to require that there be nothing in wide-int.c and everything in wide-int.h. This is a lot of code to go into a header file that is likely to be included into almost every c file. The current patch very carefully divides the implementation of each function into a fast, inlined small case that handles all types that fit in an HWI and a larger general implementation that handles the rarely used large types. That seems to have to go away to make the two different implementations of wide-int work.

4) I consider the resulting wide-int.h to be unreadable. I am sure that after the proper indoctrination period, i will just accept this glop, but to the people in the community who have some apprehension about what gcc will turn into now that C++ is allowed, this seems like the poster child of where they did not want this to go. The amount of c++ trickery employed here for what seems like i truly marginal gain seems to be out of proportion.

Mike and I gave this a good try, but in the end i think that it is better to not do this change. I get it that having only one storage class is not going to give you a medium term large int rep. But there are other ways to go
to get this. We can simply define a class that is a medium term holder and copy the values into it. Again, the copying is not that bad because it is almost always 1 element.


Kenny


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]