Do not eagerly add guarded leaf classes with CHA; do not add them for private names.

Do not eagerly add guarded leaf classes with CHA; do not add them for private names.

BUG=
R=rmacnak@google.com

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

[srdjan, 2015-05-28 22:10:45]

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

[rmacnak, 2015-05-28 23:16:53]

LGTM w/C

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h
File runtime/vm/cha.h (right):

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode35
runtime/vm/cha.h:35: // Updates set of leaf classes that we register optimized
code with for lazy
No longer true.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode41
runtime/vm/cha.h:41: // Updates set of leaf classes that we register optimized
code with for lazy
No longer true.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode46
runtime/vm/cha.h:46: // Updates set of leaf classes that we register optimized
code with for lazy
No longer true.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode57
runtime/vm/cha.h:57: void AddToLeafClasses(const Class& cls);
Add a note that this does not include all leaf classes but only classes were we
made an optimization on the assumption that the class has no
subclasses/implementors.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_o...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:2290:
(callee_receiver->AsParameter()->index() == 0)) {
If we are inside a closure function, parameter 0 is the closure, not the
receiver. But there should be no way for a closure to call itself via parameter
0, so ASSERT(!function.IsClosureFunction());

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_t...
File runtime/vm/flow_graph_type_propagator.cc (right):

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_t...
runtime/vm/flow_graph_type_propagator.cc:771: (function.IsDynamicFunction() ||
function.IsGenerativeConstructor())) {
And not closure function.

[srdjan, 2015-05-28 23:28:57]

Thanks!

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h
File runtime/vm/cha.h (right):

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode35
runtime/vm/cha.h:35: // Updates set of leaf classes that we register optimized
code with for lazy
On 2015/05/28 23:16:52, rmacnak wrote:
> No longer true.

Removed.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode41
runtime/vm/cha.h:41: // Updates set of leaf classes that we register optimized
code with for lazy
On 2015/05/28 23:16:52, rmacnak wrote:
> No longer true.

Removed.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode46
runtime/vm/cha.h:46: // Updates set of leaf classes that we register optimized
code with for lazy
On 2015/05/28 23:16:52, rmacnak wrote:
> No longer true.

Removed.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/cha.h#newcode57
runtime/vm/cha.h:57: void AddToLeafClasses(const Class& cls);
On 2015/05/28 23:16:52, rmacnak wrote:
> Add a note that this does not include all leaf classes but only classes were
we
> made an optimization on the assumption that the class has no
> subclasses/implementors.

Done.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_o...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:2290:
(callee_receiver->AsParameter()->index() == 0)) {
On 2015/05/28 23:16:52, rmacnak wrote:
> If we are inside a closure function, parameter 0 is the closure, not the
> receiver. But there should be no way for a closure to call itself via
parameter
> 0, so ASSERT(!function.IsClosureFunction());

IsDynamicFunction() returns false for closures.

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_t...
File runtime/vm/flow_graph_type_propagator.cc (right):

https://codereview.chromium.org/1158673004/diff/60001/runtime/vm/flow_graph_t...
runtime/vm/flow_graph_type_propagator.cc:771: (function.IsDynamicFunction() ||
function.IsGenerativeConstructor())) {
On 2015/05/28 23:16:53, rmacnak wrote:
> And not closure function.

IsDynamicFunction does not include closures, so it should be fine.

[srdjan, 2015-05-29 15:58:10]

Committed patchset #6 (id:100001) manually as
d7d0d92ec3bd6fc07a1b8c2d13e8e080f7f43f55 (presubmit successful).