http://codereview.chromium.org/10700115/diff/7001/src/compiler.cc File src/compiler.cc (right):
http://codereview.chromium.org/10700115/diff/7001/src/compiler.cc#newcode315 src/compiler.cc:315: LChunk* chunk = Lithium::CreateChunk(graph, &builder); How about LChunk::NewChunk? http://codereview.chromium.org/10700115/diff/7001/src/compiler.cc#newcode317 src/compiler.cc:317: Handle<Code> optimized_code = Lithium::Codegen(chunk); How about chunk->Codegen(); ? http://codereview.chromium.org/10700115/diff/7001/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/10700115/diff/7001/src/hydrogen.cc#newcode3075 src/hydrogen.cc:3075: bool HGraph::Optimize(const char** bailout_reason) { Memory management/owner is totally unclear for the bailout reason. Shouldn't you pass this via a shared string or equivalent? http://codereview.chromium.org/10700115/diff/7001/src/lithium.cc File src/lithium.cc (right): http://codereview.chromium.org/10700115/diff/7001/src/lithium.cc#newcode257 src/lithium.cc:257: LChunk* Lithium::CreateChunk(HGraph* graph, HGraphBuilder* graph_builder) { Again, adding a dependency on graph builder here is only due to the bailout, and that doesn't seem right. Pass failure reason back out. http://codereview.chromium.org/10700115/diff/7001/src/lithium.h File src/lithium.h (right): http://codereview.chromium.org/10700115/diff/7001/src/lithium.h#newcode627 src/lithium.h:627: static LChunk* CreateChunk(HGraph* graph, HGraphBuilder* builder); Why not make this a static method on LChunk? http://codereview.chromium.org/10700115/diff/7001/src/lithium.h#newcode628 src/lithium.h:628: static Handle<Code> Codegen(LChunk* chunk); Can you make this a method on the chunk? http://codereview.chromium.org/10700115/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
