Change how we deal with manual inlining of argument error and NSM when propagating types. This chan…

Change how we deal with manual inlining of argument error and NSM when propagating types. This change helps reducing bailout calls by doing more receiver and argument checks.

R=kasperl@google.com

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

[kasperl, 2013-06-14 06:19:08]

LGTM.

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/bailout.dart (right):

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:255: if (instruction
== receiver) {
Add a comment that explains what you're doing here.

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/types_propagation.dart
(right):

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/types_propagation.dart:17: :
workmap = new Map<int, HInstruction>(),
Initialize these at the declaration site.

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/types_propagation.dart:152: kind
== HTypeConversion.RECEIVER_TYPE_CHECK
I'd compute the selector before the type conversion to make this fit on fewer
lines.

https://codereview.chromium.org/16944004/diff/7001/tests/compiler/dart2js/is_...
File tests/compiler/dart2js/is_inference_test.dart (right):

https://codereview.chromium.org/16944004/diff/7001/tests/compiler/dart2js/is_...
tests/compiler/dart2js/is_inference_test.dart:11: print('foo');
So printing foo is enough to ensure that String + is in the system? Is there a
more obvious way actually involving + (in a helper method perhaps)?

If you want to keep this, I'd go for something like:

  print('Printing this ensures that String+ is in the system.');

and do that in all the other places where you print.

[ngeoffray, 2013-06-14 06:58:34]

Thanks Kasper!

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/bailout.dart (right):

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:255: if (instruction
== receiver) {
On 2013/06/14 06:19:08, kasperl wrote:
> Add a comment that explains what you're doing here.

Done.

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/types_propagation.dart
(right):

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/types_propagation.dart:17: :
workmap = new Map<int, HInstruction>(),
On 2013/06/14 06:19:08, kasperl wrote:
> Initialize these at the declaration site.

Done.

https://codereview.chromium.org/16944004/diff/7001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/types_propagation.dart:152: kind
== HTypeConversion.RECEIVER_TYPE_CHECK
On 2013/06/14 06:19:08, kasperl wrote:
> I'd compute the selector before the type conversion to make this fit on fewer
> lines.

Done.

https://codereview.chromium.org/16944004/diff/7001/tests/compiler/dart2js/is_...
File tests/compiler/dart2js/is_inference_test.dart (right):

https://codereview.chromium.org/16944004/diff/7001/tests/compiler/dart2js/is_...
tests/compiler/dart2js/is_inference_test.dart:11: print('foo');
On 2013/06/14 06:19:08, kasperl wrote:
> So printing foo is enough to ensure that String + is in the system?

Yes, the [: param + 53 :] expression line 15 does not know what param is and
could hit String+ or num+.

> Is there a
> more obvious way actually involving + (in a helper method perhaps)?

The helper method might be inlined and any use of a + in it optimized. It is a
bit more complicated if we don't want the helper to be a no-op.
> 
> If you want to keep this, I'd go for something like:
> 
>   print('Printing this ensures that String+ is in the system.');
> 
> and do that in all the other places where you print.

Done.

[ngeoffray, 2013-06-14 07:54:31]

Committed patchset #4 manually as r24019 (presubmit successful).