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

[Sanjoy Das]

Actually, I don't think there is any need for a separate smultiply
when we're working on two's complement.  IMHO, the right solution is
to just make ConstantRange::multiply smarter about handing cases like
[-1,2) * [-1,2) etc.

-- Sanjoy

On Fri, Feb 20, 2015 at 3:40 PM, Sanjoy Das
<sanjoy at playingwithpointers.com> wrote:
> 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