[interpreter] Faster and fewer flushes in register optimizer.

[interpreter] Faster and fewer flushes in register optimizer.

This change requires a single pass over the register set during
bytecode pipeline flushes.

A few bytecode tests are updated too because the order of register
flushes is different.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/ecd4086cf0dd20ea31179dffb95bb722518d8b1d
Cr-Commit-Position: refs/heads/master@{#36726}

[oth, 2016-06-02 11:08:47]

oth@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[oth, 2016-06-02 11:08:48]

Ross, PTAL. Effect on Sunspider here:

https://docs.google.com/spreadsheets/d/1A78Ek1DvSyX9-QdzE6bq5gzQt7g0DT2vDM7LO...

[rmcilroy, 2016-06-02 12:51:34]

LGTM.

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
File src/interpreter/bytecode-register-optimizer.cc (right):

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
src/interpreter/bytecode-register-optimizer.cc:219: continue;
nit - could you add a comment here that you are skipping non-materialized reg
info, and they will be materialized when a materialized equivalent is reached.

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
src/interpreter/bytecode-register-optimizer.cc:378: flush_required_ = true;
This scares me a bit since if we add another call to ddToEquivalenceSetOf
somewhere else and forget the flush_required_ things will break. Maybe add a
helper function which does both operations and call this from here?

[oth, 2016-06-02 14:14:24]

Thanks, done. This depends on the softening CL. There is a "stronger" version of
that in the pipeline that has a much more dramatic effect on code size, this
later CL might be the better one to wait for the source position testing.

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
File src/interpreter/bytecode-register-optimizer.cc (right):

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
src/interpreter/bytecode-register-optimizer.cc:219: continue;
On 2016/06/02 12:51:33, rmcilroy wrote:
> nit - could you add a comment here that you are skipping non-materialized reg
> info, and they will be materialized when a materialized equivalent is reached.

Done.

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
src/interpreter/bytecode-register-optimizer.cc:378: flush_required_ = true;
On 2016/06/02 12:51:34, rmcilroy wrote:
> This scares me a bit since if we add another call to ddToEquivalenceSetOf
> somewhere else and forget the flush_required_ things will break. Maybe add a
> helper function which does both operations and call this from here?

Done.

If flushes aren't correctly emitted many tests fail. 

We've consciously made a common path for the register transfer bytecodes so I'm
unclear why we'd ever have another path doing this operation.

[oth, 2016-06-02 14:16:14]

On 2016/06/02 12:51:34, rmcilroy wrote:
> LGTM.
> 
>
https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
> File src/interpreter/bytecode-register-optimizer.cc (right):
> 
>
https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
> src/interpreter/bytecode-register-optimizer.cc:219: continue;
> nit - could you add a comment here that you are skipping non-materialized reg
> info, and they will be materialized when a materialized equivalent is reached.
> 
>
https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
> src/interpreter/bytecode-register-optimizer.cc:378: flush_required_ = true;
> This scares me a bit since if we add another call to ddToEquivalenceSetOf
> somewhere else and forget the flush_required_ things will break. Maybe add a
> helper function which does both operations and call this from here?

^^

My comment wasn't worded very well. Trying again:

"Done, I agree it's a little scary. In mitigation, we've consciously made a
common path for the register transfer bytecodes so are unlikely to do this
elsewhere. And there are many standard test cases that fail if flushes aren't
done correctly."

[rmcilroy, 2016-06-02 15:07:13]

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
File src/interpreter/bytecode-register-optimizer.cc (right):

https://codereview.chromium.org/2033013002/diff/1/src/interpreter/bytecode-re...
src/interpreter/bytecode-register-optimizer.cc:378: flush_required_ = true;
On 2016/06/02 14:14:24, oth wrote:
> On 2016/06/02 12:51:34, rmcilroy wrote:
> > This scares me a bit since if we add another call to ddToEquivalenceSetOf
> > somewhere else and forget the flush_required_ things will break. Maybe add a
> > helper function which does both operations and call this from here?
> 
> Done.
> 
> If flushes aren't correctly emitted many tests fail. 
> 
> We've consciously made a common path for the register transfer bytecodes so
I'm
> unclear why we'd ever have another path doing this operation.

Acknowledged, I'm fine with this if you are.

[oth, 2016-06-04 09:44:08]

The CQ bit was checked by oth@chromium.org

[oth, 2016-06-04 09:44:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2033013002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-06-04 09:44:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2033013002/60001

[commit-bot: I haz the power, 2016-06-04 09:47:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-04 09:48:16]

Description was changed from

==========
[interpreter] Faster and fewer flushes in register optimizer.

This change requires a single pass over the register set during
bytecode pipeline flushes.

A few bytecode tests are updated too because the order of register
flushes is different.

BUG=v8:4280
LOG=N
==========

to

==========
[interpreter] Faster and fewer flushes in register optimizer.

This change requires a single pass over the register set during
bytecode pipeline flushes.

A few bytecode tests are updated too because the order of register
flushes is different.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/ecd4086cf0dd20ea31179dffb95bb722518d8b1d
Cr-Commit-Position: refs/heads/master@{#36726}
==========

[commit-bot: I haz the power, 2016-06-04 09:48:17]

Patchset 4 (id:??) landed as
https://crrev.com/ecd4086cf0dd20ea31179dffb95bb722518d8b1d
Cr-Commit-Position: refs/heads/master@{#36726}