FlowGraphCompiler is not a visitor any longer. Start consolidating shared code between the x64 and …

FlowGraphCompiler is not a visitor any longer. Start consolidating shared code between the x64 and ia32 compilers into a FlowGraphCompilerShared. Idea would to use the FlowGraphCompilerShared type from shared code, and FlowGraphCompiler from machine specific code.
Committed: https://code.google.com/p/dart/source/detail?r=8207

[Vyacheslav Egorov (Google), 2012-06-01 10:55:24]

lgtm

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
File runtime/vm/flow_graph_compiler_ia32.h (right):

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_ia32.h:94: Assembler* assembler_;
assembler_, block_order_, current_block_, deopt_stubs_, is_optimizing_ and all
related accessors can be shared between compilers as well

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
File runtime/vm/flow_graph_compiler_shared.cc (right):

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_shared.cc:42:
FlowGraphCompilerShared::GetBlockLabel(BlockEntryInstr* block_entry) const {
this is a very strange formatting.

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_x64.cc:758: current_block_->EmitNativeCode(this);
This is extremely confusing to read, if I did not know that current_block_ is
just current_block_entry_label_ I would thing that this call emits native code
for all instructions in the block.

Maybe add a comment.

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_x64.cc:776: __
jmp(&block_info()[successor->postorder_number()]->label);
use GetLabelFor?

[srdjan, 2012-06-01 17:44:46]

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
File runtime/vm/flow_graph_compiler_ia32.h (right):

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_ia32.h:94: Assembler* assembler_;
On 2012/06/01 10:55:24, Vyacheslav Egorov (Google) wrote:
> assembler_, block_order_, current_block_, deopt_stubs_, is_optimizing_ and all
> related accessors can be shared between compilers as well


Did not want to make the CL too big, but since you insist, you insist ...

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
File runtime/vm/flow_graph_compiler_shared.cc (right):

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_shared.cc:42:
FlowGraphCompilerShared::GetBlockLabel(BlockEntryInstr* block_entry) const {
On 2012/06/01 10:55:24, Vyacheslav Egorov (Google) wrote:
> this is a very strange formatting. 

Done.

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_x64.cc:758: current_block_->EmitNativeCode(this);
On 2012/06/01 10:55:24, Vyacheslav Egorov (Google) wrote:
> This is extremely confusing to read, if I did not know that current_block_ is
> just current_block_entry_label_ I would thing that this call emits native code
> for all instructions in the block.
> 
> Maybe add a comment.

Agreed with readability problem, needs at least a comment or a different  name
of method. Renaming: instead of EmitNativeCode -> PrepareEntry

https://chromiumcodereview.appspot.com/10447133/diff/5003/runtime/vm/flow_gra...
runtime/vm/flow_graph_compiler_x64.cc:776: __
jmp(&block_info()[successor->postorder_number()]->label);
On 2012/06/01 10:55:24, Vyacheslav Egorov (Google) wrote:
> use GetLabelFor?

Done.