Reland of [ignition/turbo] Perform liveness analysis on the bytecodes

[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.

Committed: https://crrev.com/2bf71f888fc3bcb9936e77f6aca4252034422423
Cr-Commit-Position: refs/heads/master@{#41355}

[Leszek Swirski, 2016-11-23 17:05:45]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-23 17:05:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-11-23 17:08:25]

leszeks@chromium.org changed reviewers:
+ jarin@chromium.org, rmcilroy@chromium.org

[Leszek Swirski, 2016-11-23 17:08:26]

Hi Ross and Jaro,

Here's a first pass for the bytecode liveness analysis. There's definitely
improvements to be made, some refactoring I should do (especially the big
anonymous namespace in bytecode-analysis.cc) but I wanted to get your thoughts
in case you think it's not going in the right direction.

[commit-bot: I haz the power, 2016-11-23 17:12:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-23 17:12:16]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Jarin, 2016-11-24 12:47:17]

Overall direction looking good, except the LivenessHelper thing - we are not big
fans of template magic.

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
src/compiler/bytecode-analysis.cc:179: OperandType... operandsTypes>
Oh, my eyes, my eyes!

Does the heavy use of templates help anything? Is it really noticeably slower
using the methods from the Bytecodes class?

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
File src/compiler/bytecode-analysis.h (right):

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
src/compiler/bytecode-analysis.h:50: LivenessMap;
Does this offer significant advantage over std::unordered_map?

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-graph...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:110: NodeVector tmp_values_;
Nit: Could we give this a more descriptive name (checkpoint_values_buffer_ or
some such) or describe in a comment what this is about?

https://codereview.chromium.org/2523893003/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-accessor.h (right):

https://codereview.chromium.org/2523893003/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-accessor.h:51: std::ostream& Output(std::ostream&
os) const;
Nit: Elsewhere, this is called Print(...) or PrintTo(...). Could you use the
same name?

[Leszek Swirski, 2016-11-25 15:15:54]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 15:15:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-25 15:30:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 15:30:48]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Leszek Swirski, 2016-11-25 17:27:32]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 17:27:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-11-25 17:31:27]

Hi Ross and Jaro,

Here's a big updated version, almost entirely changed since the first one. No
more template magic (sad face), and some performance boosts by being a bit more
intelligent about which bytecodes to analyse on subsequent passes. PTAL.

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
src/compiler/bytecode-analysis.cc:179: OperandType... operandsTypes>
On 2016/11/24 12:47:17, Jarin wrote:
> Oh, my eyes, my eyes!
> 
> Does the heavy use of templates help anything? Is it really noticeably slower
> using the methods from the Bytecodes class?

Beauty is in the eye of the beholder ;)

Unfortunately, as much as I love template stuff like this, both you and Ross
(f2f) don't, and I can't justify it in terms of performance because it doesn't
seem to make a difference. Removed.

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
File src/compiler/bytecode-analysis.h (right):

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-analy...
src/compiler/bytecode-analysis.h:50: LivenessMap;
On 2016/11/24 12:47:17, Jarin wrote:
> Does this offer significant advantage over std::unordered_map?

I haven't measured here, but I've measured advantages over std::unordered_map
elsewhere (in the ConstantArrayBuilder).

Might be worth revisiting, but I want to think harder about the data structure
here anyway (in a subsequent CL) because I think we can take advantage of the
fact that offsets are reasonably dense and often accessed in order.

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-graph...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2523893003/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:110: NodeVector tmp_values_;
On 2016/11/24 12:47:17, Jarin wrote:
> Nit: Could we give this a more descriptive name (checkpoint_values_buffer_ or
> some such) or describe in a comment what this is about?

Done.

https://codereview.chromium.org/2523893003/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-accessor.h (right):

https://codereview.chromium.org/2523893003/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-accessor.h:51: std::ostream& Output(std::ostream&
os) const;
On 2016/11/24 12:47:17, Jarin wrote:
> Nit: Elsewhere, this is called Print(...) or PrintTo(...). Could you use the
> same name?

Done.

[commit-bot: I haz the power, 2016-11-25 17:34:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 17:34:54]

Dry run: Try jobs failed on following builders:
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)

[Jarin, 2016-11-28 08:34:43]

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:15: namespace compiler {
Why does this stuff have to live in a header?

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:24: // statement in a called function)
won't keep locals alive.
I am not sure what you are arguing for. We do want to make debugging awesome but
minimize the impact if we are not debugging . Making stuff alive for debugger
statement is ok because debugger is slow anyway (and seeing your locals is
good). Making arbitrary calls slow is not ok.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:36: for (int i = 0; i < num_operands;
++i) {
Aaaaah, no templates, beauuuuuuutiful.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:47: if (!r.is_parameter()) {
Out of curiousity, do we ever have output register pair that stores into
parameters?

If not, then we could perhaps DCHECK here.

I am asking because this assumes that r not being a parameter implies r+1 is
also not a parameter, so it bakes in some assumptions about register numbering.
The DCHECK would not really fix it, but it would make it more obvious what we
expect.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:123: // guarantee that its header is valid
yet. Save it for later.
I am wondering whether it really matters that the header is not fully correct.

The header definitely contains all the contributions of this loop, and you have
already added the contributions of the outer loops. As a result, I cannot see
what new information can be added by another pass.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-graph-builder.cc:467: // PrintF("\n");
Remove the comments?

[rmcilroy, 2016-11-28 10:00:31]

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:15: namespace compiler {
On 2016/11/28 08:34:43, Jarin wrote:
> Why does this stuff have to live in a header?

+1. Could this not all just live in bytecode analysis.h ?

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:89: interpreter::Register r =
accessor.GetRegisterOperand(i);
nit - maybe you could add a GetRegisterListOperand which returns a RegisterList.
You could also do this for RegPair/RegTriple and add a Add(RegList) method to
the liveness map which would simplify the code a bit.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:91: if (!r.is_parameter()) {
Same comment as Jaro above regarding parameters. Maybe you could DCHECK that if
one is a parameter then they all are?

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:128: if (!Bytecodes::IsJump(bytecode)
|| Bytecodes::IsConditionalJump(bytecode)) {
Nit - just add IsUnconditionalJump in bytecodes.h

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:53: current_offset,
bytecode_array()->register_count() + 1, zone());
nit - add a comment that the + 1 is for the accumulator. Another option - add
methods to liveness_map explicitly for the accumulator, since it should always
be being accessed independently of the registers - that way there wouldn't be an
implicit assumption that register_count() is the accumulator - WDYT?

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:78: // ensured those loops are valid), we can
safely pass update the loop end and
nit - "pass update the loop end" seems like it's missing a word.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-graph-builder.cc:654: }
nit - could you move this to a method on BytecodeAnalysis?

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-graph-builder.cc:658: OFStream of(stdout);
unused?

https://codereview.chromium.org/2523893003/diff/120001/src/interpreter/byteco...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:743:
loop_builder->LoopHeader(&resume_points_in_loop);
Why did you need to change this code here?

[Leszek Swirski, 2016-11-28 10:11:02]

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:15: namespace compiler {
On 2016/11/28 08:34:43, Jarin wrote:
> Why does this stuff have to live in a header?

I suppose it needn't, I moved it here while it was still full of template magic.
Moving back.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:24: // statement in a called function)
won't keep locals alive.
On 2016/11/28 08:34:43, Jarin wrote:
> I am not sure what you are arguing for. We do want to make debugging awesome
but
> minimize the impact if we are not debugging . Making stuff alive for debugger
> statement is ok because debugger is slow anyway (and seeing your locals is
> good). Making arbitrary calls slow is not ok.

I guess what I'm arguing is that this is a leaky abstraction -- there are a
myriad of ways in which you can break in the middle of an optimised function,
and what's happening here is that one of those ways has different behaviour to
others, which is inconsistent and possibly surprising. Indeed, we turn off the
liveness analysis in a couple of debugger tests, but not in others, because it
sometimes breaks them and sometimes doesn't.

That said, I do acknowledge that *sometimes* presenting good debug information
is better than *never*. I'm just wondering if

a) doing so at the cost of inconsistency is the right thing to do, and
b) special casing single statement liveness for a single bytecode is the right
way of doing it, vs. e.g. disabling liveness entirely for functions with
debugger statements, or even not optimising functions with debugger statements
at all.

Let me know what you think.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:36: for (int i = 0; i < num_operands;
++i) {
On 2016/11/28 08:34:43, Jarin wrote:
> Aaaaah, no templates, beauuuuuuutiful.

:(

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:47: if (!r.is_parameter()) {
On 2016/11/28 08:34:43, Jarin wrote:
> Out of curiousity, do we ever have output register pair that stores into
> parameters?
> 
> If not, then we could perhaps DCHECK here.
> 
> I am asking because this assumes that r not being a parameter implies r+1 is
> also not a parameter, so it bakes in some assumptions about register
numbering.
> The DCHECK would not really fix it, but it would make it more obvious what we
> expect.

Good point, I do make the assumption that lists of registers are either entirely
parameters or entirely not. I've added DCHECKs to make sure.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:123: // guarantee that its header is valid
yet. Save it for later.
On 2016/11/28 08:34:43, Jarin wrote:
> I am wondering whether it really matters that the header is not fully correct.
> 
> The header definitely contains all the contributions of this loop, and you
have
> already added the contributions of the outer loops. As a result, I cannot see
> what new information can be added by another pass.

The problem is, I need to add this new liveness information to the loop header
before I can read it for the JumpLoop, and I can't simply force-update the
target of the JumpLoop because that's not necessarily the Jump that jumps out of
the loop (plus, there can be breaks inside the loop too) -- and we can't simply
force-add these additional liveness bits to the JumpLoop, because they may have
been killed as part of the loop header. Consider

1: do {
2:   x1 = r1;
2:   x2 = r2;
3:   do {
4:     r1 = y;
5:     break;
6:   } while(true);
7: } while(true);

r1 should be live at 2, 4 and 5, but should be dead at 1, 3 and 6, while r2 is
live throughout. On the second pass, we first need to add the liveness in 7
(propagated from 2) to 5,4 and 3, before we can process the liveness of 6 (which
makes r2 live but keeps r1 dead).

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-graph-builder.cc:467: // PrintF("\n");
On 2016/11/28 08:34:43, Jarin wrote:
> Remove the comments?

Oops, left overs from dev, I'd thought I'd caught them all. Done.

[Leszek Swirski, 2016-11-28 11:07:22]

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:89: interpreter::Register r =
accessor.GetRegisterOperand(i);
On 2016/11/28 10:00:30, rmcilroy wrote:
> nit - maybe you could add a GetRegisterListOperand which returns a
RegisterList.
> You could also do this for RegPair/RegTriple and add a Add(RegList) method to
> the liveness map which would simplify the code a bit.

I'm not sure, liveness here is a raw BitVector so for this change I'd have to
wrap it in something first and then wire through the various accessors. Do you
think this is worth it?

Also, I like that the register and register count access is separated out here,
because it makes it obvious why i is incremented -- if we were to make a change
like this, I'd also want to add an operand iterator which keeps this safe.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:91: if (!r.is_parameter()) {
On 2016/11/28 10:00:30, rmcilroy wrote:
> Same comment as Jaro above regarding parameters. Maybe you could DCHECK that
if
> one is a parameter then they all are?

Done.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:128: if (!Bytecodes::IsJump(bytecode)
|| Bytecodes::IsConditionalJump(bytecode)) {
On 2016/11/28 10:00:30, rmcilroy wrote:
> Nit - just add IsUnconditionalJump in bytecodes.h

Done, also gave me the chance to simplify it a bit.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:53: current_offset,
bytecode_array()->register_count() + 1, zone());
On 2016/11/28 10:00:30, rmcilroy wrote:
> nit - add a comment that the + 1 is for the accumulator. Another option - add
> methods to liveness_map explicitly for the accumulator, since it should always
> be being accessed independently of the registers - that way there wouldn't be
an
> implicit assumption that register_count() is the accumulator - WDYT?

Added a comment. As with another reply, adding methods for this sort of thing
would require a wrapper around the liveness BitVector, so I don't know if it's
worth doing.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:78: // ensured those loops are valid), we can
safely pass update the loop end and
On 2016/11/28 10:00:31, rmcilroy wrote:
> nit - "pass update the loop end" seems like it's missing a word.

Done -- it was too many words rather than too few.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-graph-builder.cc:654: }
On 2016/11/28 10:00:31, rmcilroy wrote:
> nit - could you move this to a method on BytecodeAnalysis?

Done. There's some code duplication inside BytecodeAnalysis now between
PrintLivenessTo and LivenessIsValid, but the latter does some more heavy-duty
printing to make debugging easier, and I'm hesitant to include that in
non-release code. LMKWYT.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-graph-builder.cc:658: OFStream of(stdout);
On 2016/11/28 10:00:31, rmcilroy wrote:
> unused?

Another debugging left over, thanks.

https://codereview.chromium.org/2523893003/diff/120001/src/interpreter/byteco...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:743:
loop_builder->LoopHeader(&resume_points_in_loop);
On 2016/11/28 10:00:31, rmcilroy wrote:
> Why did you need to change this code here?

Oh, this was a drive-by fix -- I had changed LoopBuilder::LoopHeader to allow
nullptr so that I could use it in tests, and I noticed that we could
fast-path/clarify the non-generator case here and avoid initialising a
ZoneVector and risk allocating anything on the zone (afaik in most
implementations vectors are constructed without an initial allocation, but it
doesn't hurt to be careful).

[Leszek Swirski, 2016-11-28 11:22:41]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 11:22:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-28 11:34:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-28 11:34:37]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Leszek Swirski, 2016-11-28 11:49:24]

leszeks@chromium.org changed reviewers:
+ yangguo@chromium.org

[Leszek Swirski, 2016-11-28 11:49:24]

+yangguo: Could we get your input on the discussion about what to do with
debugger bytecodes? Thanks.

[Jarin, 2016-11-28 12:05:42]

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:24: // statement in a called function)
won't keep locals alive.
On 2016/11/28 10:11:01, Leszek Swirski wrote:
> On 2016/11/28 08:34:43, Jarin wrote:
> > I am not sure what you are arguing for. We do want to make debugging awesome
> but
> > minimize the impact if we are not debugging . Making stuff alive for
debugger
> > statement is ok because debugger is slow anyway (and seeing your locals is
> > good). Making arbitrary calls slow is not ok.
> 
> I guess what I'm arguing is that this is a leaky abstraction -- there are a
> myriad of ways in which you can break in the middle of an optimised function,
> and what's happening here is that one of those ways has different behaviour to
> others, which is inconsistent and possibly surprising. Indeed, we turn off the
> liveness analysis in a couple of debugger tests, but not in others, because it
> sometimes breaks them and sometimes doesn't.
> 
> That said, I do acknowledge that *sometimes* presenting good debug information
> is better than *never*. I'm just wondering if
> 
> a) doing so at the cost of inconsistency is the right thing to do, and
> b) special casing single statement liveness for a single bytecode is the right
> way of doing it, vs. e.g. disabling liveness entirely for functions with
> debugger statements, or even not optimising functions with debugger statements
> at all.
> 
> Let me know what you think.

I talked to Yang and he is happy not to keep variables alive for the debugger
statement.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:123: // guarantee that its header is valid
yet. Save it for later.
On 2016/11/28 10:11:01, Leszek Swirski wrote:
> On 2016/11/28 08:34:43, Jarin wrote:
> > I am wondering whether it really matters that the header is not fully
correct.
> > 
> > The header definitely contains all the contributions of this loop, and you
> have
> > already added the contributions of the outer loops. As a result, I cannot
see
> > what new information can be added by another pass.
> 
> The problem is, I need to add this new liveness information to the loop header
> before I can read it for the JumpLoop, and I can't simply force-update the
> target of the JumpLoop because that's not necessarily the Jump that jumps out
of
> the loop (plus, there can be breaks inside the loop too) -- and we can't
simply
> force-add these additional liveness bits to the JumpLoop, because they may
have
> been killed as part of the loop header. Consider
> 
> 1: do {
> 2:   x1 = r1;
> 2:   x2 = r2;
> 3:   do {
> 4:     r1 = y;
> 5:     break;
> 6:   } while(true);
> 7: } while(true);
> 
> r1 should be live at 2, 4 and 5, but should be dead at 1, 3 and 6, while r2 is
> live throughout. On the second pass, we first need to add the liveness in 7
> (propagated from 2) to 5,4 and 3, before we can process the liveness of 6
(which
> makes r2 live but keeps r1 dead).

You are right. It is unfortunate that it has to be so complicated, but it seems
unavoidable.

[Leszek Swirski, 2016-11-28 12:44:12]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 12:44:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-11-28 12:57:55]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 12:58:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-28 13:24:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-28 13:24:54]

Dry run: This issue passed the CQ dry run.

[Leszek Swirski, 2016-11-28 14:17:26]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 14:17:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-11-28 14:18:51]

Ready for another round of reviews, PTAL

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:24: // statement in a called function)
won't keep locals alive.
On 2016/11/28 12:05:42, Jarin wrote:
> On 2016/11/28 10:11:01, Leszek Swirski wrote:
> > On 2016/11/28 08:34:43, Jarin wrote:
> > > I am not sure what you are arguing for. We do want to make debugging
awesome
> > but
> > > minimize the impact if we are not debugging . Making stuff alive for
> debugger
> > > statement is ok because debugger is slow anyway (and seeing your locals is
> > > good). Making arbitrary calls slow is not ok.
> > 
> > I guess what I'm arguing is that this is a leaky abstraction -- there are a
> > myriad of ways in which you can break in the middle of an optimised
function,
> > and what's happening here is that one of those ways has different behaviour
to
> > others, which is inconsistent and possibly surprising. Indeed, we turn off
the
> > liveness analysis in a couple of debugger tests, but not in others, because
it
> > sometimes breaks them and sometimes doesn't.
> > 
> > That said, I do acknowledge that *sometimes* presenting good debug
information
> > is better than *never*. I'm just wondering if
> > 
> > a) doing so at the cost of inconsistency is the right thing to do, and
> > b) special casing single statement liveness for a single bytecode is the
right
> > way of doing it, vs. e.g. disabling liveness entirely for functions with
> > debugger statements, or even not optimising functions with debugger
statements
> > at all.
> > 
> > Let me know what you think.
> 
> I talked to Yang and he is happy not to keep variables alive for the debugger
> statement.

Ok, I'll update the relevant tests to disable liveness.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:123: // guarantee that its header is valid
yet. Save it for later.
On 2016/11/28 12:05:42, Jarin wrote:
> On 2016/11/28 10:11:01, Leszek Swirski wrote:
> > On 2016/11/28 08:34:43, Jarin wrote:
> > > I am wondering whether it really matters that the header is not fully
> correct.
> > > 
> > > The header definitely contains all the contributions of this loop, and you
> > have
> > > already added the contributions of the outer loops. As a result, I cannot
> see
> > > what new information can be added by another pass.
> > 
> > The problem is, I need to add this new liveness information to the loop
header
> > before I can read it for the JumpLoop, and I can't simply force-update the
> > target of the JumpLoop because that's not necessarily the Jump that jumps
out
> of
> > the loop (plus, there can be breaks inside the loop too) -- and we can't
> simply
> > force-add these additional liveness bits to the JumpLoop, because they may
> have
> > been killed as part of the loop header. Consider
> > 
> > 1: do {
> > 2:   x1 = r1;
> > 2:   x2 = r2;
> > 3:   do {
> > 4:     r1 = y;
> > 5:     break;
> > 6:   } while(true);
> > 7: } while(true);
> > 
> > r1 should be live at 2, 4 and 5, but should be dead at 1, 3 and 6, while r2
is
> > live throughout. On the second pass, we first need to add the liveness in 7
> > (propagated from 2) to 5,4 and 3, before we can process the liveness of 6
> (which
> > makes r2 live but keeps r1 dead).
> 
> You are right. It is unfortunate that it has to be so complicated, but it
seems
> unavoidable.

Since this was quite a tricky example, I've added a test case for it. It's not
impossible that there's still a way to optimise things here.

[commit-bot: I haz the power, 2016-11-28 14:43:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-28 14:43:09]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-11-28 15:16:37]

A couple of suggestions and some ideas for followups, but other than that LGTM
assuming Jaro is happy, thanks.

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:89: interpreter::Register r =
accessor.GetRegisterOperand(i);
On 2016/11/28 11:07:22, Leszek Swirski wrote:
> On 2016/11/28 10:00:30, rmcilroy wrote:
> > nit - maybe you could add a GetRegisterListOperand which returns a
> RegisterList.
> > You could also do this for RegPair/RegTriple and add a Add(RegList) method
to
> > the liveness map which would simplify the code a bit.
> 
> I'm not sure, liveness here is a raw BitVector so for this change I'd have to
> wrap it in something first and then wire through the various accessors. Do you
> think this is worth it?

I guess we are fine without it. Please add some comments somewhere though that
the accumulator is stored in the last element of the bitvector

> Also, I like that the register and register count access is separated out
here,
> because it makes it obvious why i is incremented -- if we were to make a
change
> like this, I'd also want to add an operand iterator which keeps this safe.

You could keep the Register and Count parameters seperate, but construct a
RegList out of reg and count and use it for a seperate Add(RegList) (and the
pair / triple)?

Also, I agree that it's nice to make it clear why i is incremented, however it's
not super clear given the if (!r.is_parameter()). Maybe just unconditinally do
GetRegisterCountOperand(++i) but only update liveness when it's not a parameter
?

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:37: }
I guess you can drop this now?

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:130: }
ditto

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:153: int handler_offset =
bytecode_array->LookupRangeInHandlerTable(
(side thought) - I wonder if this ends up being expensive to do every time.
Might be worth investigating whether we can cache handler table information or
access it more efficiently in a followup CL?

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:240: // Skip bytecodes until we hit a
jumploop. This check isn't needed for
nit - JumpLoop

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:256: // Go past loop end.
nit - comment is slightly confusing. How about "Advance into the loop body"?

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:282: // Keep the iterator going so that we can
find other loops.
As mentioned offline, I'm not super keen on the last_invalid_jumploop_offset and
keeping going to find other loops. Once you add a random access iterator (as a
followup CL), it would be good to keep a stack of JumpLoop offsets found in the
first pass, and just do the second pass on each of those loops in backwards
order.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:467: loop_indent++;
Could you share code with PrintLivenessTo here?

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.h (right):

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.h:39: const BitVector* GetInLivenessFor(int
offset) const;
nit - comments for these functions

[Leszek Swirski, 2016-11-28 16:31:11]

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis-liveness.h (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis-liveness.h:89: interpreter::Register r =
accessor.GetRegisterOperand(i);
On 2016/11/28 15:16:37, rmcilroy wrote:
> On 2016/11/28 11:07:22, Leszek Swirski wrote:
> > On 2016/11/28 10:00:30, rmcilroy wrote:
> > > nit - maybe you could add a GetRegisterListOperand which returns a
> > RegisterList.
> > > You could also do this for RegPair/RegTriple and add a Add(RegList) method
> to
> > > the liveness map which would simplify the code a bit.
> > 
> > I'm not sure, liveness here is a raw BitVector so for this change I'd have
to
> > wrap it in something first and then wire through the various accessors. Do
you
> > think this is worth it?
> 
> I guess we are fine without it. Please add some comments somewhere though that
> the accumulator is stored in the last element of the bitvector

Done.

> > Also, I like that the register and register count access is separated out
> here,
> > because it makes it obvious why i is incremented -- if we were to make a
> change
> > like this, I'd also want to add an operand iterator which keeps this safe.
> 
> You could keep the Register and Count parameters seperate, but construct a
> RegList out of reg and count and use it for a seperate Add(RegList) (and the
> pair / triple)?

The wrapper class comment still stands here, I'll look into it in a separate CL.

> Also, I agree that it's nice to make it clear why i is incremented, however
it's
> not super clear given the if (!r.is_parameter()). Maybe just unconditinally do
> GetRegisterCountOperand(++i) but only update liveness when it's not a
parameter
> ?

Done.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:37: }
On 2016/11/28 15:16:37, rmcilroy wrote:
> I guess you can drop this now?

Done.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:130: }
On 2016/11/28 15:16:37, rmcilroy wrote:
> ditto

Done.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:153: int handler_offset =
bytecode_array->LookupRangeInHandlerTable(
On 2016/11/28 15:16:37, rmcilroy wrote:
> (side thought) - I wonder if this ends up being expensive to do every time.
> Might be worth investigating whether we can cache handler table information or
> access it more efficiently in a followup CL? 

Yes, I've got this in the back of my head as one of the "easy" speedups we could
look at -- I didn't focus on it in this CL because exceptions tend to be slower
anyway. I'll add a TODO.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:240: // Skip bytecodes until we hit a
jumploop. This check isn't needed for
On 2016/11/28 15:16:37, rmcilroy wrote:
> nit - JumpLoop

Done.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:256: // Go past loop end.
On 2016/11/28 15:16:37, rmcilroy wrote:
> nit - comment is slightly confusing. How about "Advance into the loop body"?

Done.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:282: // Keep the iterator going so that we can
find other loops.
On 2016/11/28 15:16:37, rmcilroy wrote:
> As mentioned offline, I'm not super keen on the last_invalid_jumploop_offset
and
> keeping going to find other loops. Once you add a random access iterator (as a
> followup CL), it would be good to keep a stack of JumpLoop offsets found in
the
> first pass, and just do the second pass on each of those loops in backwards
> order. 

Yup, follow-up CL incoming, I just need to make sure that the random access code
doesn't regress anything (it shouldn't, but I still need to check).

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:467: loop_indent++;
On 2016/11/28 15:16:37, rmcilroy wrote:
> Could you share code with PrintLivenessTo here?

I potentially could, but there's some additional machinery here to  highlight
the incorrect liveness vector, and to draw loops, which I'm not super keen on
adding to 'PrintLivenessTo', since I'd have to hide them behind parameters.

I'd be more tempted to merge both this and 'PrintLivenessTo' with
BytecodeArray::Disassemble, and have a single bytecode printing mega-function
that can annotate statements with liveness, draw loops, maybe  draw forward
jumps, and whatever else we dream of. But I'd rather punt that to another CL.

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.h (right):

https://codereview.chromium.org/2523893003/diff/220001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.h:39: const BitVector* GetInLivenessFor(int
offset) const;
On 2016/11/28 15:16:37, rmcilroy wrote:
> nit - comments for these functions

Done (also the location of the accumulator comment).

[Jarin, 2016-11-28 19:47:20]

lgtm, can't way to see this in action!

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
File src/compiler/bytecode-analysis.cc (right):

https://codereview.chromium.org/2523893003/diff/120001/src/compiler/bytecode-...
src/compiler/bytecode-analysis.cc:123: // guarantee that its header is valid
yet. Save it for later.
On 2016/11/28 14:18:51, Leszek Swirski wrote:
> On 2016/11/28 12:05:42, Jarin wrote:
> > On 2016/11/28 10:11:01, Leszek Swirski wrote:
> > > On 2016/11/28 08:34:43, Jarin wrote:
> > > > I am wondering whether it really matters that the header is not fully
> > correct.
> > > > 
> > > > The header definitely contains all the contributions of this loop, and
you
> > > have
> > > > already added the contributions of the outer loops. As a result, I
cannot
> > see
> > > > what new information can be added by another pass.
> > > 
> > > The problem is, I need to add this new liveness information to the loop
> header
> > > before I can read it for the JumpLoop, and I can't simply force-update the
> > > target of the JumpLoop because that's not necessarily the Jump that jumps
> out
> > of
> > > the loop (plus, there can be breaks inside the loop too) -- and we can't
> > simply
> > > force-add these additional liveness bits to the JumpLoop, because they may
> > have
> > > been killed as part of the loop header. Consider
> > > 
> > > 1: do {
> > > 2:   x1 = r1;
> > > 2:   x2 = r2;
> > > 3:   do {
> > > 4:     r1 = y;
> > > 5:     break;
> > > 6:   } while(true);
> > > 7: } while(true);
> > > 
> > > r1 should be live at 2, 4 and 5, but should be dead at 1, 3 and 6, while
r2
> is
> > > live throughout. On the second pass, we first need to add the liveness in
7
> > > (propagated from 2) to 5,4 and 3, before we can process the liveness of 6
> > (which
> > > makes r2 live but keeps r1 dead).
> > 
> > You are right. It is unfortunate that it has to be so complicated, but it
> seems
> > unavoidable.
> 
> Since this was quite a tricky example, I've added a test case for it. It's not
> impossible that there's still a way to optimise things here.

Thanks!

[Leszek Swirski, 2016-11-29 09:41:55]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-11-29 09:41:56]

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/2523893003/#ps300001
(title: "Address Ross's comments")

[commit-bot: I haz the power, 2016-11-29 09:42:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-29 09:44:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-29 09:44:35]

Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/13342)

[Leszek Swirski, 2016-11-29 10:05:57]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-11-29 10:05:57]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org,
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2523893003/#ps320001
(title: "Access handler table directly")

[commit-bot: I haz the power, 2016-11-29 10:06:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-29 10:46:00]

CQ is committing da patch.

Bot data: {"patchset_id": 320001, "attempt_start_ts": 1480413957181750,
"parent_rev": "790ff5cf2a95e16296224d090e780d845ee0b8b5", "commit_rev":
"d4a8317068595b9b16c580744fe75c2e7b16ab35"}

[commit-bot: I haz the power, 2016-11-29 10:46:09]

Committed patchset #17 (id:320001)

[commit-bot: I haz the power, 2016-11-29 10:46:27]

Description was changed from

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.
==========

to

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.

Committed: https://crrev.com/1852300954c216c29cf93444430681d213e87925
Cr-Commit-Position: refs/heads/master@{#41344}
==========

[commit-bot: I haz the power, 2016-11-29 10:46:29]

Patchset 17 (id:??) landed as
https://crrev.com/1852300954c216c29cf93444430681d213e87925
Cr-Commit-Position: refs/heads/master@{#41344}

[Leszek Swirski, 2016-11-29 10:51:05]

A revert of this CL (patchset #17 id:320001) has been created in
https://codereview.chromium.org/2541443002/ by leszeks@chromium.org.

The reason for reverting is: Breaks the build:

https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20shared/build....

[Leszek Swirski, 2016-11-29 11:20:39]

Description was changed from

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.

Committed: https://crrev.com/1852300954c216c29cf93444430681d213e87925
Cr-Commit-Position: refs/heads/master@{#41344}
==========

to

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.

Committed: https://crrev.com/1852300954c216c29cf93444430681d213e87925
Cr-Commit-Position: refs/heads/master@{#41344}
==========

[Leszek Swirski, 2016-11-29 11:21:33]

Description was changed from

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.

Committed: https://crrev.com/1852300954c216c29cf93444430681d213e87925
Cr-Commit-Position: refs/heads/master@{#41344}
==========

to

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.
==========

[Leszek Swirski, 2016-11-29 12:00:02]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-11-29 12:00:02]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org,
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2523893003/#ps340001
(title: "Export handler table for tests")

[commit-bot: I haz the power, 2016-11-29 12:00:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-29 12:26:55]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1480420802591350,
"parent_rev": "56362c7e746f27263d596052d69c9bb87e2243dc", "commit_rev":
"1aefa5b77e8a3641a21de3f1046660414aa9f494"}

[commit-bot: I haz the power, 2016-11-29 12:26:59]

Description was changed from

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.
==========

to

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.
==========

[commit-bot: I haz the power, 2016-11-29 12:27:00]

Committed patchset #18 (id:340001)

[commit-bot: I haz the power, 2016-11-29 12:27:23]

Description was changed from

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.
==========

to

==========
[ignition/turbo] Perform liveness analysis on the bytecodes

Replaces the graph-based liveness analyzer in the bytecode graph builder
with an initial bytecode-based liveness analysis pass, which is added to
the existing loop extent analysis.

Now the StateValues in the graph have their inputs initialised to
optimized_out, rather than being modified after the graph is built.

Committed: https://crrev.com/2bf71f888fc3bcb9936e77f6aca4252034422423
Cr-Commit-Position: refs/heads/master@{#41355}
==========

[commit-bot: I haz the power, 2016-11-29 12:27:25]

Patchset 18 (id:??) landed as
https://crrev.com/2bf71f888fc3bcb9936e77f6aca4252034422423
Cr-Commit-Position: refs/heads/master@{#41355}