Add "force inline" to internal annotations.

Add "force inline" to internal annotations.

R=johnniwinther@google.com

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

[floitsch, 2015-02-26 20:07:39]

floitsch@google.com changed reviewers:
+ herhut@google.com, johnniwinther@google.com

[Johnni Winther, 2015-03-02 10:02:28]

LGTM

What happens if we have @ForceInline on a recursive method?

https://codereview.chromium.org/962703004/diff/1/pkg/compiler/lib/src/js_back...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/962703004/diff/1/pkg/compiler/lib/src/js_back...
pkg/compiler/lib/src/js_backend/backend.dart:72: if (insideLoop == null ||
insideLoop) {
Why allow [insideLoop] to be null. Shouldn't the default value just be `true`
instead?

[floitsch, 2015-03-02 15:18:29]

https://codereview.chromium.org/962703004/diff/1/pkg/compiler/lib/src/js_back...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/962703004/diff/1/pkg/compiler/lib/src/js_back...
pkg/compiler/lib/src/js_backend/backend.dart:72: if (insideLoop == null ||
insideLoop) {
On 2015/03/02 10:02:28, Johnni Winther wrote:
> Why allow [insideLoop] to be null. Shouldn't the default value just be `true`
> instead?

Done.

[floitsch, 2015-03-02 15:40:54]

On 2015/03/02 10:02:28, Johnni Winther wrote:
> LGTM
> 
> What happens if we have @ForceInline on a recursive method?
Nothing.
The force-inlining follows a few other checks.
I could move the "force-Inline" higher up, but the heuristics look ok to me. (no
inlining into a throw expression, for example).

> 
>
https://codereview.chromium.org/962703004/diff/1/pkg/compiler/lib/src/js_back...
> File pkg/compiler/lib/src/js_backend/backend.dart (right):
> 
>
https://codereview.chromium.org/962703004/diff/1/pkg/compiler/lib/src/js_back...
> pkg/compiler/lib/src/js_backend/backend.dart:72: if (insideLoop == null ||
> insideLoop) {
> Why allow [insideLoop] to be null. Shouldn't the default value just be `true`
> instead?

[floitsch, 2015-03-02 17:25:17]

PTAL.
I had forgotten an inlining case.

[Johnni Winther, 2015-03-03 08:57:47]

https://codereview.chromium.org/962703004/diff/80001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/962703004/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_backend/backend.dart:29: static const int _mustNotInline
= 0;
The logic for these tags is both complex and scattered. Maybe the tags should be
replaced by objects that encapsulates the logic.

[floitsch, 2015-03-03 18:54:13]

PTAL. It's now bigger, but hopefully easier to follow through.

https://codereview.chromium.org/962703004/diff/80001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/962703004/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_backend/backend.dart:29: static const int _mustNotInline
= 0;
On 2015/03/03 08:57:47, Johnni Winther wrote:
> The logic for these tags is both complex and scattered. Maybe the tags should
be
> replaced by objects that encapsulates the logic.

Made the logic much more explicit (and verbose).

[Johnni Winther, 2015-03-04 09:19:16]

lgtm

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:107: break;
`break;` shouldn't be necessary for warning-freeness.

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:115: case
_mayInlineInLoopMustNotOutside:
Shouldn't _mayInlineInLoopMustNotOutside but replaced by
_canInlineInLoopMustNotOutside?

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:131: break;
`break;` shouldn't be necessary for warning-freeness.

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:182: break;
`break;` shouldn't be necessary for warning-freeness.

[floitsch, 2015-03-04 13:24:59]

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:107: break;
On 2015/03/04 09:19:16, Johnni Winther wrote:
> `break;` shouldn't be necessary for warning-freeness.

Done.

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:115: case
_mayInlineInLoopMustNotOutside:
On 2015/03/04 09:19:16, Johnni Winther wrote:
> Shouldn't _mayInlineInLoopMustNotOutside but replaced by
> _canInlineInLoopMustNotOutside? 

good catch. thanks.
done.

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:131: break;
On 2015/03/04 09:19:16, Johnni Winther wrote:
> `break;` shouldn't be necessary for warning-freeness.

Done.

https://codereview.chromium.org/962703004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:182: break;
On 2015/03/04 09:19:16, Johnni Winther wrote:
> `break;` shouldn't be necessary for warning-freeness.

Done.

[floitsch, 2015-03-04 13:37:19]

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