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

Nick Lewycky nlewycky at google.com
Fri Feb 20 18:11:55 PST 2015


On 20 February 2015 at 17:49, Nick Lewycky <nlewycky at google.com> wrote:

> On 20 February 2015 at 17:41, Sanjoy Das <sanjoy at playingwithpointers.com>
> wrote:
>
>> > 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 wasn't considering that a problem. ConstantRange::multiply
> today precisely produces the range [3, 10) in that case. There is no
> improvement to be made.
>
> Consider i8 [1, 4) * i8 [9, 10).
>

Uhm, I meant to say "i8 [1, 86) * i8 [9, 10)". That's quite a typo.

The best possible range is [1, 253), but our implementation will return the
> full set. That's really hard to get right. We extend the bit range to twice
> the bit range so that we can do the multiply algebraically, then we call
> truncate. Truncate correctly detects that we cover the whole span of
> integers in the bitwidth and returns the full set.
>
> Nick
>
> 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
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150220/6064d7fa/attachment.html>


More information about the llvm-commits mailing list