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

[Sanjoy Das]

I don't think so:

[-3,0) * [-3, 0) should be [1, 10) but using your formula I get [3,
<something>).

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