More precise range analysis for smi multiplication.

More precise range analysis for smi multiplication.

R=vegorov@google.com

Committed: https://code.google.com/p/dart/source/detail?r=37355

[Vyacheslav Egorov (Google), 2014-06-13 13:59:30]

lgtm

https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:3088: const int64_t left_max =
ConstantAbsMax(left_range);
Can you test it for kMinInt64 * -1? Because I am bit concerned about what
ConstantAbsMax(kMinInt64) returns. 

Similar for kMinSmi * -1.

[Florian Schneider, 2014-06-16 10:45:10]

https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:3088: const int64_t left_max =
ConstantAbsMax(left_range);
On 2014/06/13 13:59:29, Vyacheslav Egorov (Google) wrote:
> Can you test it for kMinInt64 * -1? Because I am bit concerned about what
> ConstantAbsMax(kMinInt64) returns. 
> 
> Similar for kMinSmi * -1.

We're staying in smi range here: ConstantAbsMax returns a value between 0 and
-kMinSmi, which is always smaller than kMaxInt.

[Florian Schneider, 2014-06-16 10:53:03]

On 2014/06/16 10:45:10, Florian Schneider wrote:
>
https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
> File runtime/vm/intermediate_language.cc (right):
> 
>
https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
> runtime/vm/intermediate_language.cc:3088: const int64_t left_max =
> ConstantAbsMax(left_range);
> On 2014/06/13 13:59:29, Vyacheslav Egorov (Google) wrote:
> > Can you test it for kMinInt64 * -1? Because I am bit concerned about what
> > ConstantAbsMax(kMinInt64) returns. 
> > 
> > Similar for kMinSmi * -1.
> 
> We're staying in smi range here: ConstantAbsMax returns a value between 0 and
> -kMinSmi, which is always smaller than kMaxInt.

I'll add an assertion to test for this.

[Florian Schneider, 2014-06-16 11:14:06]

Committed patchset #2 manually as r37355 (presubmit successful).

[Cutch, 2014-06-16 14:29:09]

On 2014/06/16 10:53:03, Florian Schneider wrote:
> On 2014/06/16 10:45:10, Florian Schneider wrote:
> >
>
https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
> > File runtime/vm/intermediate_language.cc (right):
> > 
> >
>
https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
> > runtime/vm/intermediate_language.cc:3088: const int64_t left_max =
> > ConstantAbsMax(left_range);
> > On 2014/06/13 13:59:29, Vyacheslav Egorov (Google) wrote:
> > > Can you test it for kMinInt64 * -1? Because I am bit concerned about what
> > > ConstantAbsMax(kMinInt64) returns. 
> > > 
> > > Similar for kMinSmi * -1.
> > 
> > We're staying in smi range here: ConstantAbsMax returns a value between 0
and
> > -kMinSmi, which is always smaller than kMaxInt.
> 

This isn't the case with https://codereview.chromium.org/328503003/

[Florian Schneider, 2014-06-16 14:50:27]

On 2014/06/16 14:29:09, Cutch wrote:
> On 2014/06/16 10:53:03, Florian Schneider wrote:
> > On 2014/06/16 10:45:10, Florian Schneider wrote:
> > >
> >
>
https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
> > > File runtime/vm/intermediate_language.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/336773003/diff/1/runtime/vm/intermediate_lang...
> > > runtime/vm/intermediate_language.cc:3088: const int64_t left_max =
> > > ConstantAbsMax(left_range);
> > > On 2014/06/13 13:59:29, Vyacheslav Egorov (Google) wrote:
> > > > Can you test it for kMinInt64 * -1? Because I am bit concerned about
what
> > > > ConstantAbsMax(kMinInt64) returns. 
> > > > 
> > > > Similar for kMinSmi * -1.
> > > 
> > > We're staying in smi range here: ConstantAbsMax returns a value between 0
> and
> > > -kMinSmi, which is always smaller than kMaxInt.
> > 
> 
> This isn't the case with https://codereview.chromium.org/328503003/

You're right. The check for overflow becomes slightly more complicated in this
case.