[PATCH] [ConstantRange] Add a smultiply method for signed multiply

[Hal Finkel]

----- Original Message -----
> From: "Sanjoy Das" <sanjoy at playingwithpointers.com>
> To: reviews+D7789+public+ffa7549bb8a7b441 at reviews.llvm.org
> Cc: "james molloy" <james.molloy at arm.com>, "Hal Finkel" <hfinkel at anl.gov>, "Commit Messages and Patches for LLVM"
> <llvm-commits at cs.uiuc.edu>
> Sent: Friday, February 20, 2015 5:11:46 PM
> Subject: Re: [PATCH] [ConstantRange] Add a smultiply method for signed multiply
> 
> >  ConstantRange
> > +ConstantRange::smultiply(const ConstantRange &Other) const {
> > +  // TODO: If either operand is a single element and the multiply
> > is known to
> > +  // be non-wrapping, round the result min and max value to the
> > appropriate
> > +  // multiple of that element. If wrapping is possible, at least
> > adjust the
> > +  // range according to the greatest power-of-two factor of the
> > single element.
> > +
> > +  if (isEmptySet() || Other.isEmptySet())
> > +    return ConstantRange(getBitWidth(), /*isFullSet=*/false);
> > +
> > +  APInt this_min = getSignedMin().sext(getBitWidth() * 2);
> > +  APInt this_max = getSignedMax().sext(getBitWidth() * 2);
> > +  APInt Other_min = Other.getSignedMin().sext(getBitWidth() * 2);
> > +  APInt Other_max = Other.getSignedMax().sext(getBitWidth() * 2);
> > +
> > +  ConstantRange Result_zext = ConstantRange(this_min * Other_min,
> > +                                            this_max * Other_max +
> > 1);
> > +  return Result_zext.truncate(getBitWidth());
> 
> I'm not sure if this is correct:  if we multiply [-1, 2) with itself,
> won't this return [1, 2)?  Shouldn't [-1,2) * [-1,2) be [-1,2)?

Indeed. The minimum should not be this_min * Other_min, but rather min(this_min * Other_max, this_max * Other_min), right?

 -Hal

> 
> -- Sanjoy
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory