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]

Re: patch to fix constant math - 4th patch - the wide-int class - patch ping for the next stage 1


Yes, I agree that you win the challenge that it can be done. What you have always failed to address is why anyone would want to do this. Or how this would at all be desirable. But I completely agree that from a purely abstract point of view you can add a storage model.

Now here is why we REALLY do not want to go down this road:

1)  The following comment from your earlier mail is completely wrong

+#ifdef NEW_REP_FOR_INT_CST
+  /* This is the code once the tree level is converted.  */
+  wide_int result;
+  int i;
+
+  tree type = TREE_TYPE (tcst);
+
+  result.bitsize = GET_MODE_BITSIZE (TYPE_MODE (type));
+  result.precision = TYPE_PRECISION (type);
+  result.len = TREE_INT_CST_LEN (tcst);
+  for (i = 0; i < result.len; i++)
+    result.val[i] = TREE_INT_CST_ELT (tcst, i);
+
+  return result;
+#else

this also shows the main reason I was asking for storage abstraction.
The initialization from tree is way too expensive.

In almost all cases, constants will fit in a single HWI. Thus, the only thing that you are copying is the length and a single HWI. So you are dragging in a lot of machinery just to save these two copies? Certainly there has to be more to it than that.

2) You present this as if the implementor actually should care about the implementation and you give 3 alternatives: the double_int, the current one, and HWI. We have tried to make it so that the client should not care. Certainly in my experience here, I have not found a place to care.

In my opinion double_int needs to go away. That is the main thrust of my patches. There is no place in a compiler for an abi that depends on constants fitting into 2 two words whose size is defined by the host. This is not a beauty contest argument, we have public ports are beginning to use modes that are larger than two x86-64 HWIs and i have a private port that has such modes and it is my experience that any pass that uses this interface has one of three behaviors: it silently gets the wrong answer, it ices, or it fails to do the transformation. If we leave double_int as an available option, then any use of it potentially will have one of these three behaviors. And so one of my strong objections to this direction is that i do not want to fight this kind of bug for the rest of my life. Having a single storage model that just always works is in my opinion a highly desirable option. What you have never answered in a concrete manner is, if we decide to provide this generality, what it would be used for. There is no place in a portable compiler where the right answer for every target is two HOST wide integers.

However, i will admit that the HWI option has some merits. We try to address this in our implementation by dividing what is done inline in wide-int.h to the cases that fit in an HWI and then only drop into the heavy code in wide-int.c if mode is larger (which it rarely will be). However, a case could be made that for certain kinds of things like string lengths and such, we could use another interface or as you argue, a different storage model with the same interface. I just do not see that the cost of the conversion code is really going to show up on anyone's radar.

3) your trick will work at the tree level, but not at the rtl level. The wide-int code cannot share storage with the CONST_INTs. We tried this, and there are a million bugs that would have to be fixed to make it work. It could have worked if CONST_INTs had carried a mode around, but since they do not, you end up with the same CONST_INT sharing the rep for several different types and that just did not work unless you are willing to do substantial cleanups.

On 04/02/2013 11:04 AM, Richard Biener wrote:
On Wed, Feb 27, 2013 at 2:59 AM, Kenneth Zadeck
<zadeck@naturalbridge.com> wrote:
This patch contains a large number of the changes requested by Richi.   It
does not contain any of the changes that he requested to abstract the
storage layer.   That suggestion appears to be quite unworkable.
I of course took this claim as a challenge ... with the following result.  It is
of course quite workable ;)

The attached patch implements the core wide-int class and three storage
models (fixed size for things like plain HWI and double-int, variable size
similar to how your wide-int works and an adaptor for the double-int as
contained in trees).  With that you can now do

HOST_WIDE_INT
wi_test (tree x)
{
   // template argument deduction doesn't do the magic we want it to do
   // to make this kind of implicit conversions work
   // overload resolution considers this kind of conversions so we
   // need some magic that combines both ... but seeding the overload
   // set with some instantiations doesn't seem to be possible :/
   // wide_int<> w = x + 1;
   wide_int<> w;
   w += x;
   w += 1;
   // template argument deduction doesn't deduce the return value type,
   // not considering the template default argument either ...
   // w = wi (x) + 1;
   // we could support this by providing rvalue-to-lvalue promotion
   // via a traits class?
   // otoh it would lead to sub-optimal code anyway so we should
   // make the result available as reference parameter and only support
   // wide_int <> res; add (res, x, 1); ?
   w = wi (x).operator+<wide_int<> >(1);
   wide_int<>::add(w, x, 1);
   return w.to_hwi ();
}

we are somewhat limited with C++ unless we want to get really fancy.
Eventually providing operator+ just doesn't make much sense for
generic wide-int combinations (though then the issue is its operands
are no longer commutative which I think is the case with your wide-int
or double-int as well - they don't suport "1 + wide_int" for obvious reasons).

So there are implementation design choices left undecided.

Oh, and the operation implementations are crap (they compute nonsense).

But you should get the idea.

Richard.


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