This is the mail archive of the 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: [wide-int] int_traits <tree>

On 10/16/2013 09:55 AM, Richard Biener wrote:
Speaking in terms of a patch:

Index: tree.h
--- tree.h      (revision 203690)
+++ tree.h      (working copy)
@@ -5184,14 +5184,11 @@ wi::int_traits <const_tree>::decompose (
                           / HOST_BITS_PER_WIDE_INT);
    unsigned int xprecision = get_precision (x);
- gcc_assert (precision >= xprecision);
+  gcc_checking_assert (precision >= xprecision && precision != 0);
- /* Got to be careful of precision 0 values. */
-  if (precision)
-    len = MIN (len, max_len);
-      unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
+      unsigned int small_prec = xprecision & (HOST_BITS_PER_WIDE_INT -
        if (small_prec)
           /* We have to futz with this because the canonization for
@@ -5201,7 +5198,7 @@ wi::int_traits <const_tree>::decompose (
               for (unsigned int i = 0; i < len - 1; i++)
                 scratch[i] = val[i];
-             scratch[len - 1] = sext_hwi (val[len - 1], precision);
+             scratch[len - 1] = sext_hwi (val[len - 1], small_prec);
               return wi::storage_ref (scratch, len, precision);

the precision 0 thing is gone as far as I understand?
There is still the case where the front ends create types with precision 0, and that causes a few odd cases, but the precision 0 that you refer to is gone.
Also sign-extending the upper hwi must be done with precision %
HOST_BITS_PER_WIDE_INT, no?  And it assumes that we only ever
have to extend the upper most HWI, thus 'len' is never too big.
I note that we immediately return in the above case, so

       if (precision < xprecision + HOST_BITS_PER_WIDE_INT)
           len = wi::force_to_size (scratch, val, len, xprecision,
precision, UNSIGNED);
           return wi::storage_ref (scratch, len, precision);

applies only when the desired precision is a multiple of a HWI.
I assume it adds that extra zero word in case the MSB is set,
I am confused about the condition written here and
how we look at precision and not xprecision when deciding how
to extend - given a 8-bit precision unsigned 0xff and precision == 64
we do not even consider sign-extending the value because we look
at precision and not xprecision.  Don't we need to look at
xprecision here?
I do not think so.   There are three cases here:

1) xprecision < precision:
say xprecision = 8 and precision = 32.

The number is between 0 and 255 and after canonicalization the number will be between 0 and 255.

2) xprecision == precision
not much to talk about here.

3) xprecision > precision

We need to loose the upper bits of the input and it does not matter what they were. The only thing that we are concerned about is that the bits above what is now the sign bit pointed to by precision are matched in the bits above precision.

I do think we could change is that the "if (precision < xprecision + HWI)" to become "else if (...)", which will save a test.
After all an assert precision == xprecision
does not work in this routine.

Quickly execute.exp tested only.

To avoid code quality wreckage we have to implement a different way
of allocating the 'scratch' storage of wide_int_ref_storage
(well, ideally we wouldn't need it ...).  I thought of simply
allocating it via alloca (), but we need to do that in all
callers that build a wide_int_ref (eventually via a hidden
default arg).  But as that's a template specialization of
generic_wide_int ... so the option is to provide a function
wrapper inside wi:: for this, like
I want richard and mike to be the people who respond to the next point. I am not a c++ person and all of this storage manager layer stuff gives me a headache.

template <typename T>
wi::ref (const T &t, HOST_WIDE_INT *scratch = XALLOCAVEC (HOST_WIDE_INT,
   return wide_int_ref (t, scratch);

and amend the storage constructors to get a scratch argument.

The reason for all this is that otherwise the wide_int_ref object
escapes when we pass the scratch storage to the workers in
int_traits <const_tree>.


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