Anton, thanks for the comments. On Thu, Jan 27, 2011 at 3:00 PM, <[email protected]> wrote:
> http://codereview.chromium.org/6390003/diff/1/src/hydrogen.cc#newcode3524 > src/hydrogen.cc:3524: int context_chain_length = > should we refactor the code that builds context values chain into a > helper method? > That seems like a good idea. > > > http://codereview.chromium.org/6390003/diff/1/src/ia32/lithium-codegen-ia32.cc > File src/ia32/lithium-codegen-ia32.cc (right): > > > http://codereview.chromium.org/6390003/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode1934 > src/ia32/lithium-codegen-ia32.cc:1934: Operand(context, > Context::SlotOffset(Context::FCONTEXT_INDEX))); > maybe introduce additional instruction for this load not to repeat on > each slot load/store? > I'm about as sure as I could be that we don't need this at all, even in unoptimized code. There used to be a TODO here to figure out if it was necessary and get rid of it. We just fetched this context from a JSFunction. > > > http://codereview.chromium.org/6390003/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode2184 > src/ia32/lithium-codegen-ia32.cc:2184: __ mov(result, esi); > Can we make it noop by requesting the result register to be esi? Not yet, because esi isn't available as an allocatable register. Eventually, it will not be automatically be in esi, so it will already be in some register or else we'll have to rematerialize it. It's a drag to have to rematerialize it at a use, but right now we rematerialize it right after most calls and those will go away. -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
