[WIP][turbofan] Instruction scheduler for Turbofan.

[turbofan] Instruction scheduler for Turbofan.

Implement machine instruction scheduling after instruction selection.

Currently only works for arm64.

R=danno@chromium.org, bmeurer@chromium.org, titzer@chromium.org

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

[baptiste.afsa1, 2015-09-30 14:35:02]

Hi guys,

This is the patch for instruction scheduling on arm64. This is not ready for
commit as we still need to fix those instructions definition lists!

PTAL

[Benedikt Meurer, 2015-10-26 11:25:11]

bmeurer@chromium.org changed reviewers:
+ jarin@chromium.org
- danno@chromium.org, titzer@chromium.org

[Jarin, 2015-10-26 15:12:35]

Very nice! The code needs a bit better explanations in comments, but that is
expected in a prototype.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/arm64/instruct...
File src/compiler/arm64/instruction-codes-arm64.h (right):

https://codereview.chromium.org/1375253002/diff/1/src/compiler/arm64/instruct...
src/compiler/arm64/instruction-codes-arm64.h:133: V(Arm64StoreWriteBarrier,
kHasSideEffect)
As discussed offline, it would be better the flags separate (and have a
platform-specific function that implements the opcode -> flag mapping with a
switch).

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
File src/compiler/instruction-scheduler.cc (right):

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:11: const int
InstructionScheduler::instruction_flags_[] = {
I think Chrome cannot have static variables. How about a static function with a
big switch-case?

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:98: for (auto node : graph_) {
nit: If you went from the end of the graph, then you would not need the
recursion in ComputeTotalLatency, no? (Assuming you would compute total latency
unconditionally here.)

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:111: if (cycle >=
(*iterator)->start_cycle()) {
Could you add a comment here explaining that you only consider instructions that
have all operands ready here and among those you pick the one with the longest
critical path to the end (or something to that effect)?

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:162: // TODO(bafsa): Do we need to look
for anti-dependencies/output-dependencies?
I think we don't because SSA (although I am not entirely sure about phis in
loops).

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:173: // basic block. We need to find a
more reliable way to catch those.
Why can't they be reordered? These should be unreachable code.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
File src/compiler/instruction-scheduler.h (right):

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.h:33: // be scheduled before B.
You might want to say that it means A is a dependency of B. (And possibly also
explain that dependencies of N are unscheduled predessors of N because the
terminology is a bit confusing here.)

Alternatively, you might avoid the term "Dependency" altogether and call it
"UnscheduledPredecessor".

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.h:57: ZoneVector<ScheduleGraphNode*>
successors_;
I think it is better to use ZoneDeque here.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.h:64: // The sum of all the latencies from a
node to the root of the graph.
What is the "root of the graph" here? From the code it looks like the root is
the end of the graph. Please, clarify the comment.

[Jarin, 2015-10-26 15:41:42]

On 2015/10/26 15:12:35, Jarin wrote:
> Very nice! The code needs a bit better explanations in comments, but that is
> expected in a prototype.
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/arm64/instruct...
> File src/compiler/arm64/instruction-codes-arm64.h (right):
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/arm64/instruct...
> src/compiler/arm64/instruction-codes-arm64.h:133: V(Arm64StoreWriteBarrier,
> kHasSideEffect)
> As discussed offline, it would be better the flags separate (and have a
> platform-specific function that implements the opcode -> flag mapping with a
> switch).
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> File src/compiler/instruction-scheduler.cc (right):
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.cc:11: const int
> InstructionScheduler::instruction_flags_[] = {
> I think Chrome cannot have static variables. How about a static function with
a
> big switch-case?
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.cc:98: for (auto node : graph_) {
> nit: If you went from the end of the graph, then you would not need the
> recursion in ComputeTotalLatency, no? (Assuming you would compute total
latency
> unconditionally here.)
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.cc:111: if (cycle >=
> (*iterator)->start_cycle()) {
> Could you add a comment here explaining that you only consider instructions
that
> have all operands ready here and among those you pick the one with the longest
> critical path to the end (or something to that effect)?
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.cc:162: // TODO(bafsa): Do we need to look
> for anti-dependencies/output-dependencies?
> I think we don't because SSA (although I am not entirely sure about phis in
> loops).
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.cc:173: // basic block. We need to find a
> more reliable way to catch those.
> Why can't they be reordered? These should be unreachable code.
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> File src/compiler/instruction-scheduler.h (right):
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.h:33: // be scheduled before B.
> You might want to say that it means A is a dependency of B. (And possibly also
> explain that dependencies of N are unscheduled predessors of N because the
> terminology is a bit confusing here.)
> 
> Alternatively, you might avoid the term "Dependency" altogether and call it
> "UnscheduledPredecessor".
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.h:57: ZoneVector<ScheduleGraphNode*>
> successors_;
> I think it is better to use ZoneDeque here.
> 
>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.h:64: // The sum of all the latencies from
a
> node to the root of the graph.
> What is the "root of the graph" here? From the code it looks like the root is
> the end of the graph. Please, clarify the comment.

I should add that we have to be careful about performance. For example, in the
mandreel benchmark in octane there is one massive function (called global_init)
with many thousands call in one basic block. I am wondering how this scheduling
algorithm performs on that function.

For the standard emscripten benchmark, could you measure the effect of this on
compilation time (output of --turbo-stats)?

[Jarin, 2015-10-27 09:02:31]

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
File src/compiler/instruction-scheduler.cc (right):

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:80: }
This loop is worrisome: for a basic block with n instructions with side-effect
we get O(n^2) iterations and, more worryingly, O(n^2) edges in the graph. Could
not we just link like this:

- a side-effect node to the most recent side effect node and to all the loads
since the last side-effect.

- a load node to the most recent side-effect node.

Ideally, we could just remember the last side-effect node and the set of loads
since the last side effect. If we could quickly find all the operands, we would
not need this loop over all the existing nodes in the graph at all.

[baptiste.afsa1, 2015-10-27 16:00:24]

I'll will regenerate some data including compilation time that I'll share with
you.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/arm64/instruct...
File src/compiler/arm64/instruction-codes-arm64.h (right):

https://codereview.chromium.org/1375253002/diff/1/src/compiler/arm64/instruct...
src/compiler/arm64/instruction-codes-arm64.h:133: V(Arm64StoreWriteBarrier,
kHasSideEffect)
On 2015/10/26 15:12:35, Jarin wrote:
> As discussed offline, it would be better the flags separate (and have a
> platform-specific function that implements the opcode -> flag mapping with a
> switch).

Done.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
File src/compiler/instruction-scheduler.cc (right):

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:11: const int
InstructionScheduler::instruction_flags_[] = {
On 2015/10/26 15:12:35, Jarin wrote:
> I think Chrome cannot have static variables. How about a static function with
a
> big switch-case? 

Done.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:80: }
On 2015/10/27 09:02:31, Jarin wrote:
> This loop is worrisome: for a basic block with n instructions with side-effect
> we get O(n^2) iterations and, more worryingly, O(n^2) edges in the graph.
Could
> not we just link like this:
> 
> - a side-effect node to the most recent side effect node and to all the loads
> since the last side-effect.
> 
> - a load node to the most recent side-effect node.
> 
> Ideally, we could just remember the last side-effect node and the set of loads
> since the last side effect. If we could quickly find all the operands, we
would
> not need this loop over all the existing nodes in the graph at all.

Yes, good idea. I'll fix this.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:98: for (auto node : graph_) {
On 2015/10/26 15:12:35, Jarin wrote:
> nit: If you went from the end of the graph, then you would not need the
> recursion in ComputeTotalLatency, no? (Assuming you would compute total
latency
> unconditionally here.)

I would also need to be able to go from a node to its predecessors. As I don't
need these links to do the scheduling, they are not maintained in the graph. I'm
not sure it's worth adding them only to help ComputeTotalLatency.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:111: if (cycle >=
(*iterator)->start_cycle()) {
On 2015/10/26 15:12:35, Jarin wrote:
> Could you add a comment here explaining that you only consider instructions
that
> have all operands ready here and among those you pick the one with the longest
> critical path to the end (or something to that effect)?

Done.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:162: // TODO(bafsa): Do we need to look
for anti-dependencies/output-dependencies?
On 2015/10/26 15:12:35, Jarin wrote:
> I think we don't because SSA (although I am not entirely sure about phis in
> loops).

It was my initial thought but I wasn't too sure.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.cc:173: // basic block. We need to find a
more reliable way to catch those.
On 2015/10/26 15:12:35, Jarin wrote:
> Why can't they be reordered? These should be unreachable code.

It triggers an assertion in the register allocator.

Allowing these nops to be reordered sometime leads to the call to the runtime
function doing the throw to become the last instruction in the basic block and
it can make the DCHECK(successor->PredecessorCount() == 1) assertion in
ConstraintBuilder::MeetRegisterConstraintsForLastInstructionInBlock fail.

I agree this solution is a hack and could add some extra constraints on the
scheduler. By having a second look at this issue, I might be able to go around
it by making these nops dependent on the result of the call to the runtime
function. I don't know if this would be an acceptable solution. Any thoughts on
that?

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
File src/compiler/instruction-scheduler.h (right):

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.h:33: // be scheduled before B.
On 2015/10/26 15:12:35, Jarin wrote:
> You might want to say that it means A is a dependency of B. (And possibly also
> explain that dependencies of N are unscheduled predessors of N because the
> terminology is a bit confusing here.)
> 
> Alternatively, you might avoid the term "Dependency" altogether and call it
> "UnscheduledPredecessor".

Done.

Good idea. Using the term UnscheduledPredecessor does make things clearer.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.h:57: ZoneVector<ScheduleGraphNode*>
successors_;
On 2015/10/26 15:12:35, Jarin wrote:
> I think it is better to use ZoneDeque here.

Done.

https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
src/compiler/instruction-scheduler.h:64: // The sum of all the latencies from a
node to the root of the graph.
On 2015/10/26 15:12:35, Jarin wrote:
> What is the "root of the graph" here? From the code it looks like the root is
> the end of the graph. Please, clarify the comment.

Done.

[Jarin, 2015-10-30 12:22:43]

On 2015/10/27 16:00:24, baptiste.afsa1 wrote:
> I'll will regenerate some data including compilation time that I'll share with
> you.

That would be great, thanks. Also, you might want to upload the patchset 
with all the fixes.

>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.cc:98: for (auto node : graph_) {
> On 2015/10/26 15:12:35, Jarin wrote:
> > nit: If you went from the end of the graph, then you would not need the
> > recursion in ComputeTotalLatency, no? (Assuming you would compute total
> latency
> > unconditionally here.)
> 
> I would also need to be able to go from a node to its predecessors. As I don't
> need these links to do the scheduling, they are not maintained in the graph.
I'm
> not sure it's worth adding them only to help ComputeTotalLatency.

That is not what I meant, I should have been clearer. I meant that you could go
through the reversed list of nodes so that total latency would be already
computed
for all the successors and ComputeTotalLatency would not recurse (recursion
might be 
a problem for big basic blocks).

>
https://codereview.chromium.org/1375253002/diff/1/src/compiler/instruction-sc...
> src/compiler/instruction-scheduler.cc:173: // basic block. We need to find a
> more reliable way to catch those.
> On 2015/10/26 15:12:35, Jarin wrote:
> > Why can't they be reordered? These should be unreachable code.
> 
> It triggers an assertion in the register allocator.
> 
> Allowing these nops to be reordered sometime leads to the call to the runtime
> function doing the throw to become the last instruction in the basic block and
> it can make the DCHECK(successor->PredecessorCount() == 1) assertion in
> ConstraintBuilder::MeetRegisterConstraintsForLastInstructionInBlock fail.
> 
> I agree this solution is a hack and could add some extra constraints on the
> scheduler. By having a second look at this issue, I might be able to go around
> it by making these nops dependent on the result of the call to the runtime
> function. I don't know if this would be an acceptable solution. Any thoughts
on
> that?

Ah, you have probably hit some hack on our side. It looks like it might have
something 
to do with our abuse of call - call can be a normal instruction in the middle of

a basic block (if its exception cannot be caught in the current function) or 
a block terminator (if there is an exception handler). A cleaner way might be to
introduce a special opcode for this block terminator so that you can avoid the
brittle logic that figures out what ArchNop is.

[baptiste.afsa1, 2015-11-23 15:11:17]

Hi Jarin,

I uploaded a new patch set for the instruction scheduler. It should address all
your concerns and also add support for arm32, ia32 and x64 platforms.

I'll also send you some up-to-date benchmark results soon.

Baptiste

[Jarin, 2015-11-24 11:43:04]

This is looking awesome!

The only thing that really needs fixing is the flags on ia32 and x64. It would
be also good to see the performance numbers - if ia32 and x64 does not benefit
then we might just skip instruction scheduling there.

Thanks!

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/instructio...
File src/compiler/instruction-scheduler.h (right):

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/instructio...
src/compiler/instruction-scheduler.h:53: // Record that we have scheduled one of
the predecessor of this node.
predecessor -> predecessors.

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/instructio...
src/compiler/instruction-scheduler.h:122: bool IsLiveInNop(const Instruction*
instr) const {
Funny name. Could we invent a better name? For example IsFixedRegisterParameter.


(Yes, it is unfortunate that we misused kArchNop for this.)

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/x64/instru...
File src/compiler/x64/instruction-scheduler-x64.cc (right):

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/x64/instru...
src/compiler/x64/instruction-scheduler-x64.cc:120: // stack slots?
Yeah, these instructions can actually load/store arbitrary memory, depending on
their addressing mode. I talked to Benedikt about this, and for now he
recommends to check the addressing mode (instr->addressing_mode()): if it is
none (kMode_None, I think) then it is side-effect free; if it is anything else,
then mark it as having a full side-effect. Differentiating between full
side-effect and load would require closer look at each instruction.

https://codereview.chromium.org/1375253002/diff/20001/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/1375253002/diff/20001/src/globals.h#newcode38
src/globals.h:38: #endif
I think it would be better to remove this and instead introduce
instruction-scheduler-xyz.cc for each architecture.

[baptiste.afsa1, 2015-12-01 09:08:43]

Hi Jarin,

I've uploaded a new patch set which should address the issues from your last
review.

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/instructio...
File src/compiler/instruction-scheduler.h (right):

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/instructio...
src/compiler/instruction-scheduler.h:53: // Record that we have scheduled one of
the predecessor of this node.
On 2015/11/24 11:43:03, Jarin wrote:
> predecessor -> predecessors.

Done.

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/instructio...
src/compiler/instruction-scheduler.h:122: bool IsLiveInNop(const Instruction*
instr) const {
On 2015/11/24 11:43:03, Jarin wrote:
> Funny name. Could we invent a better name? For example
IsFixedRegisterParameter.
> 
> 
> (Yes, it is unfortunate that we misused kArchNop for this.)

Done.

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/x64/instru...
File src/compiler/x64/instruction-scheduler-x64.cc (right):

https://codereview.chromium.org/1375253002/diff/20001/src/compiler/x64/instru...
src/compiler/x64/instruction-scheduler-x64.cc:120: // stack slots?
On 2015/11/24 11:43:03, Jarin wrote:
> Yeah, these instructions can actually load/store arbitrary memory, depending
on
> their addressing mode. I talked to Benedikt about this, and for now he
> recommends to check the addressing mode (instr->addressing_mode()): if it is
> none (kMode_None, I think) then it is side-effect free; if it is anything
else,
> then mark it as having a full side-effect. Differentiating between full
> side-effect and load would require closer look at each instruction.

Done.

https://codereview.chromium.org/1375253002/diff/20001/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/1375253002/diff/20001/src/globals.h#newcode38
src/globals.h:38: #endif
On 2015/11/24 11:43:03, Jarin wrote:
> I think it would be better to remove this and instead introduce
> instruction-scheduler-xyz.cc for each architecture.

Done.

[Jarin, 2015-12-02 08:27:04]

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

[commit-bot: I haz the power, 2015-12-02 08:27:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1375253002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1375253002/40001

[Jarin, 2015-12-02 08:27:49]

lgtm; however, I would still like to see the performance numbers.

https://codereview.chromium.org/1375253002/diff/40001/src/compiler/ppc/instru...
File src/compiler/ppc/instruction-scheduler-ppc.cc (right):

https://codereview.chromium.org/1375253002/diff/40001/src/compiler/ppc/instru...
src/compiler/ppc/instruction-scheduler-ppc.cc:10: 
I am wondering whether it would not be better if we had something like

bool InstructionScheduler::SchedulerSupported() { return false; }

and then query this method in the instruction selector (in addition to checking
the flag). That way, when we flip the flag to enable scheduling, it will not
blow up on the architectures that do not support scheduling.

What do you think?

[commit-bot: I haz the power, 2015-12-02 08:30:21]

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

[commit-bot: I haz the power, 2015-12-02 08:30:22]

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

[baptiste.afsa1, 2015-12-02 13:01:12]

Hi Jarin,

I've uploaded a new patch set. It includes the change you suggested about adding
a SchedulerSupported() method and a bunch of fixes which should help with all
the failures in the automated tests.

Baptiste

[Jarin, 2015-12-02 13:03:12]

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

[commit-bot: I haz the power, 2015-12-02 13:03:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1375253002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1375253002/60001

[commit-bot: I haz the power, 2015-12-02 13:04:41]

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

[commit-bot: I haz the power, 2015-12-02 13:04:42]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel/builds/12615)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/12622)
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/8405)

[Jarin, 2015-12-02 13:09:46]

On 2015/12/02 13:01:12, baptiste.afsa1 wrote:
> Hi Jarin,
> 
> I've uploaded a new patch set. It includes the change you suggested about
adding
> a SchedulerSupported() method and a bunch of fixes which should help with all
> the failures in the automated tests.
> 
> Baptiste

There is some redness, it looks like you will have to rebase once more. Which
reminds me - next time, could you please upload the rebases separately from your
changes? Otherwise, your changes get mixed up with the rebase changes, and then
it is necessary to go through the entire CL (rather than just the latest patch).

[baptiste.afsa1, 2015-12-02 13:16:51]

On 2015/12/02 13:09:46, Jarin wrote:
> On 2015/12/02 13:01:12, baptiste.afsa1 wrote:
> > Hi Jarin,
> > 
> > I've uploaded a new patch set. It includes the change you suggested about
> adding
> > a SchedulerSupported() method and a bunch of fixes which should help with
all
> > the failures in the automated tests.
> > 
> > Baptiste
> 
> There is some redness, it looks like you will have to rebase once more. Which
> reminds me - next time, could you please upload the rebases separately from
your
> changes? Otherwise, your changes get mixed up with the rebase changes, and
then
> it is necessary to go through the entire CL (rather than just the latest
patch).

Ok, sure I'll do that. Sorry for the mess.

[baptiste.afsa1, 2015-12-02 13:25:23]

The CQ bit was checked by baptiste.afsa@arm.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-02 13:25:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1375253002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1375253002/80001

[commit-bot: I haz the power, 2015-12-02 13:40:27]

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

[commit-bot: I haz the power, 2015-12-02 13:40:28]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_compil...)
  v8_win_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel/builds/12558)

[baptiste.afsa1, 2015-12-02 14:47:54]

The CQ bit was checked by baptiste.afsa@arm.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-02 14:48:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1375253002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1375253002/100001

[commit-bot: I haz the power, 2015-12-02 15:09:32]

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

[commit-bot: I haz the power, 2015-12-02 15:09:33]

Dry run: This issue passed the CQ dry run.

[baptiste.afsa1, 2015-12-03 14:37:01]

Hi Jarin,

Just to let you know in case you didn't notice that I've uploaded a last
patchset yesterday with a bunch of fixes and it passed all the tests.

Also you should have received the performance numbers via email. Let me know if
you need anything else.

[baptiste.afsa1, 2015-12-15 10:20:40]

The CQ bit was checked by baptiste.afsa@arm.com

[baptiste.afsa1, 2015-12-15 10:20:40]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1375253002/#ps120001
(title: "rebase the patch")

[commit-bot: I haz the power, 2015-12-15 10:20:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1375253002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1375253002/120001

[commit-bot: I haz the power, 2015-12-15 10:59:59]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-12-15 11:00:32]

Description was changed from

==========
[turbofan] Instruction scheduler for Turbofan.

Implement machine instruction scheduling after instruction selection.

Currently only works for arm64.

R=danno@chromium.org, bmeurer@chromium.org, titzer@chromium.org
==========

to

==========
[turbofan] Instruction scheduler for Turbofan.

Implement machine instruction scheduling after instruction selection.

Currently only works for arm64.

R=danno@chromium.org, bmeurer@chromium.org, titzer@chromium.org

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

[commit-bot: I haz the power, 2015-12-15 11:00:33]

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

[Yang, 2015-12-15 11:26:58]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1526913002/ by yangguo@chromium.org.

The reason for reverting is: Does not compile

https://build.chromium.org/p/client.v8/builders/V8%20Arm%20-%20debug%20builde....

[Yang, 2015-12-15 11:28:30]

On 2015/12/15 11:26:58, Yang wrote:
> A revert of this CL (patchset #7 id:120001) has been created in
> https://codereview.chromium.org/1526913002/ by mailto:yangguo@chromium.org.
> 
> The reason for reverting is: Does not compile
> 
>
https://build.chromium.org/p/client.v8/builders/V8%20Arm%20-%20debug%20builde....

Why did a "[WIP]" CL end up committed anyways.