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

[Richard Biener <rguenther at suse dot de>]

On Fri, 12 Apr 2013, Jakub Jelinek wrote:

> 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?

dummy_overflow.

Ok with that change.

Thanks,
Richard.