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

[Sanjoy Das]

> It's hard to do multiplication of ConstantRanges precisely, and multiply
> does have a TODO requesting improvement. The problems start once you have
> multiplies that can wrap. What you need to solve for is the max and min of
> each of the multiplies (cartesian product) in modulo 2^n space. Because we
> need to push the resulting set into a constant range, what it looks like is
> "how far > 0 is the lowest value" and "how far < UINT_MAX is the highest
> value", with no interesting information about the values in the middle.

Just to make sure I understand you:  this problem isn't really limited
to cases that actually wrap -- i32 [1, 4) * i32 [3, 4) == i32 {3, 6,
9} and this structure is lost when we coerce the result to [3, 10).

I wonder how crazy it is to have the notion of a first-class
ConstantRange "expression" (i.e. remembering that a ConstantRange is
i32 [1, 4) * i32 [3, 4) and not eagerly flattening it).

-- Sanjoy

>
> Nick
>
>>
>> > On 20 Feb 2015, at 23:48, Sanjoy Das <sanjoy at playingwithpointers.com>
>> > wrote:
>> >
>> >> 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
>> >>
>> >
>> >
>> > -- IMPORTANT NOTICE: The contents of this email and any attachments are
>> > confidential and may also be privileged. If you are not the intended
>> > recipient, please notify the sender immediately and do not disclose the
>> > contents to any other person, use it for any purpose, or store or copy the
>> > information in any medium.  Thank you.
>> >
>> > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>> > Registered in England & Wales, Company No:  2557590
>> > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1
>> > 9NJ, Registered in England & Wales, Company No:  2548782
>> >
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits