Speculate on bitwise operators.

AOT: Speculate that bitwise operators will have Smi receivers and arguments.

Add shifts to CheckedSmiOps.

Fix invalid load forwarding across CheckedSmiOps and CheckedSmiComparisons.

R=fschneider@google.com

Committed: https://github.com/dart-lang/sdk/commit/e7040c86ebb7933ce53153abbe6e5290e678da2e

[rmacnak, 2016-11-11 17:28:55]

Description was changed from

==========
Speculate on bitwise operators.
==========

to

==========
AOT: Speculate that bitwise operators will have Smi receivers and arguments.

Add shifts to CheckedSmiOps.

Fix invalid load forwarding across CheckedSmiOps and CheckedSmiComparisons.
==========

[rmacnak, 2016-11-11 17:54:01]

rmacnak@google.com changed reviewers:
+ fschneider@google.com

[rmacnak, 2016-11-11 17:54:02]

before:
I/flutter (31900): Stock build: 1480.1µs per iteration
I/flutter (32098): Stock build: 1496.6µs per iteration
I/flutter (32322): Stock build: 1509.7µs per iteration
I/flutter (32535): Stock build: 1472.4µs per iteration
I/flutter ( 1067): Stock build: 1488.0µs per iteration

after:
I/flutter (10564): Stock build: 1418.6µs per iteration
I/flutter (11047): Stock build: 1406.4µs per iteration
I/flutter (11288): Stock build: 1400.1µs per iteration
I/flutter (11531): Stock build: 1416.0µs per iteration
I/flutter (11755): Stock build: 1385.7µs per iteration

+5.977116184%


(Flutter gallery, release, arm32)
before
VMIsolate(CodeSize): 103914
Isolate(CodeSize): 1881635
ReadOnlyData(CodeSize): 1569760
Instructions(CodeSize): 4964416
Total(CodeSize): 8519725

after
VMIsolate(CodeSize): 103914
Isolate(CodeSize): 1880410
ReadOnlyData(CodeSize): 1583056
Instructions(CodeSize): 5006624 +0.85%
Total(CodeSize): 8574004  +0.64%

[Florian Schneider, 2016-11-11 18:12:20]

LGTM

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:3271: __ mov(result, Operand(left, LSL,
TMP));
Can Assembler::LsL be used here as well?

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:3285: __ mov(result, Operand(TMP, ASR,
result));
Use Asr?

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_x64.cc (right):

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
runtime/vm/intermediate_language_x64.cc:2984: __ j(LESS,
slow_path->entry_label());
Can you use unsigned compare "right <
(unsigned)Smi::RawValue(kMaxSmiShiftCount)" to save one comparison? (applies to
other platforms as well)

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/redundancy_e...
File runtime/vm/redundancy_elimination.cc (right):

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/redundancy_e...
runtime/vm/redundancy_elimination.cc:1051: instr->IsCheckedSmiComparison() ||
Good catch!

[rmacnak, 2016-11-11 21:29:02]

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:3271: __ mov(result, Operand(left, LSL,
TMP));
On 2016/11/11 18:12:20, Florian Schneider wrote:
> Can Assembler::LsL be used here as well?

Done.

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:3285: __ mov(result, Operand(TMP, ASR,
result));
On 2016/11/11 18:12:20, Florian Schneider wrote:
> Use Asr?

Done.

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_x64.cc (right):

https://codereview.chromium.org/2487873003/diff/80001/runtime/vm/intermediate...
runtime/vm/intermediate_language_x64.cc:2984: __ j(LESS,
slow_path->entry_label());
On 2016/11/11 18:12:20, Florian Schneider wrote:
> Can you use unsigned compare "right <
> (unsigned)Smi::RawValue(kMaxSmiShiftCount)" to save one comparison? (applies
to
> other platforms as well)

Yes. As discussed offline this means something like 1 >> 100 would go to the
slow path, but we expect this is rare.

[rmacnak, 2016-11-11 21:33:48]

Description was changed from

==========
AOT: Speculate that bitwise operators will have Smi receivers and arguments.

Add shifts to CheckedSmiOps.

Fix invalid load forwarding across CheckedSmiOps and CheckedSmiComparisons.
==========

to

==========
AOT: Speculate that bitwise operators will have Smi receivers and arguments.

Add shifts to CheckedSmiOps.

Fix invalid load forwarding across CheckedSmiOps and CheckedSmiComparisons.

R=fschneider@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e7040c86ebb7933ce53153abbe6e5290e678da2e
==========

[rmacnak, 2016-11-11 21:33:49]

Committed patchset #7 (id:120001) manually as
e7040c86ebb7933ce53153abbe6e5290e678da2e (presubmit successful).

[Vyacheslav Egorov (Google), 2016-11-15 21:58:34]

vegorov@google.com changed reviewers:
+ vegorov@google.com

[Vyacheslav Egorov (Google), 2016-11-15 21:58:34]

DBC

https://codereview.chromium.org/2487873003/diff/120001/runtime/vm/redundancy_...
File runtime/vm/redundancy_elimination.cc (right):

https://codereview.chromium.org/2487873003/diff/120001/runtime/vm/redundancy_...
runtime/vm/redundancy_elimination.cc:1007: instr->IsCheckedSmiComparison() ||
We should consider making this a flag / function on a Instruction. 

If we decide to add a new instruction with a slow path that can call arbitrary
Dart code we might forget to add a rule here again.