With --noopt run unoptimized code through optimizer, more optimizations can be done later.

With --noopt run unoptimized code through optimizer, more optimizations can be done later.

BUG=
R=fschneider@google.com

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

[srdjan, 2015-05-20 18:18:35]

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

[Florian Schneider, 2015-05-21 14:51:05]

LGTM.

Please make sure that we pass all our tests with --noopt, since we don't have
buildbot coverage for this mode.

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

https://codereview.chromium.org/1149713002/diff/20001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:96: current_iterator_ = ⁢
I don't think you need to use the current_iterator_ here since no instructions
are added/removed during iteration.

https://codereview.chromium.org/1149713002/diff/20001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:4206: if ((op_kind == Token::kGET) &&
TryInlineInstanceGetter(instr)) {
How do we guarantee that TryInline... does not introduce a check?

Maybe introduce a separate inline-pass (e.g. TryInlineGetterWithoutCheck)
for always-optimize that does not emit check instructions at all. Ideally it
would share most code with TryInlineInstanceGetter.

[srdjan, 2015-05-21 17:27:08]

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

https://codereview.chromium.org/1149713002/diff/20001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:96: current_iterator_ = ⁢
On 2015/05/21 14:51:05, Florian Schneider wrote:
> I don't think you need to use the current_iterator_ here since no instructions
> are added/removed during iteration.

Done.

https://codereview.chromium.org/1149713002/diff/20001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:4206: if ((op_kind == Token::kGET) &&
TryInlineInstanceGetter(instr)) {
On 2015/05/21 14:51:05, Florian Schneider wrote:
> How do we guarantee that TryInline... does not introduce a check?
> 
> Maybe introduce a separate inline-pass (e.g. TryInlineGetterWithoutCheck)
> for always-optimize that does not emit check instructions at all. Ideally it
> would share most code with TryInlineInstanceGetter.

Every check adds a deopt stub in case of failure; there is an assert making sure
that we do not emit deopt stubs when 'always_optimize'. It is probably safer to
add ass suggested   TryInlineGetterWithoutCheck. I am actually reusing the
method 'TryInlineGetterWithoutCheck' and passing an argument 'allow_check'.

[srdjan, 2015-05-22 23:27:30]

Additional changes:
- Do not allow conversion from StaticCallInstr to MathUnaryInstr (and for SIMD
constructors) as they require UnboxedDoubles (UnboxedDoubleInstr may cause
deoptimization, added a TODO).
- warn_on_javascript_compatibility does not work with always_megamoprhic_calls
(no ICData in megamorphic calls)
- turned  CHA off as it may  cause crashes as long as we do not have eager class
finalization. To measure with benchmarks run with --use-cha
- debugging tests do not work with --noopt.

[srdjan, 2015-05-27 21:37:13]

On 2015/05/22 23:27:30, srdjan wrote:
> Additional changes:
> - Do not allow conversion from StaticCallInstr to MathUnaryInstr (and for SIMD
> constructors) as they require UnboxedDoubles (UnboxedDoubleInstr may cause
> deoptimization, added a TODO).
> - warn_on_javascript_compatibility does not work with always_megamoprhic_calls
> (no ICData in megamorphic calls)
> - turned  CHA off as it may  cause crashes as long as we do not have eager
class
> finalization. To measure with benchmarks run with --use-cha
> - debugging tests do not work with --noopt.

Let me know if you have any more comments, otherwise I will submit it tomorrow.

[srdjan, 2015-05-28 17:06:00]

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