Kenneth Zadeck <zad...@naturalbridge.com> writes:
>>> +      vallen = canonize (val, (uvlen + 1) >> 1, prec);
>>> +
>>> +      /* Shift is not always safe to write over one of the
>>> +    operands, so we must copy.  */
>>> +      HOST_WIDE_INT tval[2 * WIDE_INT_MAX_ELTS];
>>> +      memcpy (tval, val, vallen * CHAR_BIT / HOST_BITS_PER_WIDE_INT);
>
>
>> vallen * sizeof (HOST_WIDE_INT) would be more typical.
>> But why not unpack into tval directly and avoid the copy?
> I could special case this, but the old code was not correct for odd 
> precisions.

It's not really special-casing, since the pack is already local to this block.
I.e. the patch had:

      wi_pack ((unsigned HOST_WIDE_INT *) val,
               r, uvlen);
      vallen = canonize (val, (uvlen + 1) >> 1, prec);

      /* Shift is not always safe to write over one of the
         operands, so we must copy.  */ 
      HOST_WIDE_INT tval[2 * WIDE_INT_MAX_ELTS];
      memcpy (tval, val, vallen * CHAR_BIT / HOST_BITS_PER_WIDE_INT); 
      vallen = wi::lrshift_large (val, tval, vallen, prec*2, prec, prec);

and I think it should be:

      unsigned int tvallen = (uvlen + 1) >> 1;
      HOST_WIDE_INT *tval = XALLOCAVEC (HOST_WIDE_INT, tvallen);
      wi_pack ((unsigned HOST_WIDE_INT *) tval, r, tvallen);
      tvallen = canonize (tval, tvalen, prec);
      vallen = wi::lrshift_large (val, tval, tvallen, prec * 2, prec, prec);

Thanks,
Richard

Reply via email to