Re: [PATCH] Fix MULT_HIGHPART_EXPR folding (PR tree-optimization/56918)

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Apr 11, 2013 at 09:13:17PM +0200, Marc Glisse wrote:
> On Thu, 11 Apr 2013, Jakub Jelinek wrote:
> 
> >The op1 - op2 line instead of multiplication is something that has been
> >introduced in C++ double_int changes, but without the 2 * TYPE_PRECISION
> >prec it would always return 0 or -1 anyway.
> 
> The comments on PREC in the rshift function really confused me.

Yeah, me too.  But if count == prec, the code always:
  /* Zero / sign extend all bits that are beyond the precision.  */
   
  if (count >= prec)
    {
      *hv = signmask;
      *lv = signmask;
    }          
which is definitely not what we want.  We want TYPE_PRECISION bits
of the result zero resp. sign extended into the whole double_int precision,
which for TYPE_PRECISION == HOST_BITS_PER_WIDE_INT is exactly the:
  else if ((prec - count) >= HOST_BITS_PER_WIDE_INT)
    {
      *hv &= ~((HOST_WIDE_INT) (-1) << (prec - count - HOST_BITS_PER_WIDE_INT));
      *hv |= signmask << (prec - count - HOST_BITS_PER_WIDE_INT);
    }
case, and for smaller TYPE_PRECISION it is the last case:
  else
    {
      *hv = signmask;
      *lv &= ~((unsigned HOST_WIDE_INT) (-1) << (prec - count));
      *lv |= signmask << (prec - count);
    }

> I have one question below.
> >+	{
> >+	  if (TYPE_PRECISION (type) != 2 * HOST_BITS_PER_WIDE_INT)
> >+	    return NULL_TREE;
> >+	  op1.wide_mul_with_sign (op2, false, &res, &overflow);
> 
> Why "false" and not something involving "uns"?

false was what rth used in his original code.  Looking at what is the
difference, it seems unsigned_p only matters for the computation of
overflow, and, MULT_HIGHPART_EXPR should never overflow, thus perhaps
we should either have a dummy bool overflow_dummy; there and use it
instead of &overflow, or just set overflow = false; afterwards.
Preferences?

	Jakub