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

[Hal Finkel]

----- Original Message -----
> From: "Sanjoy Das" <sanjoy at playingwithpointers.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "james molloy" <james.molloy at arm.com>, "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>,
> "reviews+d7789+public+ffa7549bb8a7b441" <reviews+D7789+public+ffa7549bb8a7b441 at reviews.llvm.org>
> Sent: Friday, February 20, 2015 5:40:58 PM
> Subject: Re: [PATCH] [ConstantRange] Add a smultiply method for signed multiply
> 
> I don't think so:
> 
> [-3,0) * [-3, 0) should be [1, 10) but using your formula I get [3,
> <something>).

Good point. The minimum could come from either the most negative times the most positive number, or the product of the least negative numbers, or the least positive numbers, etc.

 -Hal

> 
> On Fri, Feb 20, 2015 at 3:31 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > ----- 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
> 

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