Improve stores to global variables.

Improve stores to global variables.

Committed: http://code.google.com/p/v8/source/detail?r=3921

[Kevin Millikin (Chromium), 2010-02-19 13:11:20]

LGTM after comments are addressed.

http://codereview.chromium.org/650028/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/650028/diff/1/2#newcode692
src/ia32/codegen-ia32.cc:692: // If eax is free, the virtual frame will load the
global object directly
The frame doesn't really load the global, the code generator does.  The register
allocator will prefer eax if it's free.

http://codereview.chromium.org/650028/diff/1/2#newcode4314
src/ia32/codegen-ia32.cc:4314: // If eax is free, the virtual frame will load
the global object directly
Same.  Frame is not involved in selecting a register as long as there are
registers free.

http://codereview.chromium.org/650028/diff/1/2#newcode4782
src/ia32/codegen-ia32.cc:4782: LoadGlobal();
This doesn't really belong here but down below.

http://codereview.chromium.org/650028/diff/1/2#newcode4814
src/ia32/codegen-ia32.cc:4814: if (is_trivial_receiver) {
if (var != NULL) {
  frame()->Spill(eax);
  LoadGlobal();
} else if (is_trivial_receiver) {
  frame()->Push(prop->obj());
} else {
  frame()->Dup();
}

http://codereview.chromium.org/650028/diff/1/2#newcode4846
src/ia32/codegen-ia32.cc:4846: answer = EmitNamedGlobalStore(name);
I don't really like the name GlobalStore, because not all global stores go
through it.  EmitContextualStore?

It's sort of annoying to have a special case for global variables in the code
generator just to plumb it all the way down to the virtual frame.

Maybe it would be better to pass an is_contextual flag to EmitNamedStore,
analogous to EmitNamedLoad.

[William Hesse, 2010-02-19 14:35:18]

All comments addressed.

http://codereview.chromium.org/650028/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/650028/diff/1/2#newcode692
src/ia32/codegen-ia32.cc:692: // If eax is free, the virtual frame will load the
global object directly
On 2010/02/19 13:11:21, Kevin Millikin wrote:
> The frame doesn't really load the global, the code generator does.  The
register
> allocator will prefer eax if it's free.

Done.

http://codereview.chromium.org/650028/diff/1/2#newcode4314
src/ia32/codegen-ia32.cc:4314: // If eax is free, the virtual frame will load
the global object directly
On 2010/02/19 13:11:21, Kevin Millikin wrote:
> Same.  Frame is not involved in selecting a register as long as there are
> registers free.

Done.

http://codereview.chromium.org/650028/diff/1/2#newcode4782
src/ia32/codegen-ia32.cc:4782: LoadGlobal();
On 2010/02/19 13:11:21, Kevin Millikin wrote:
> This doesn't really belong here but down below.

Done.

http://codereview.chromium.org/650028/diff/1/2#newcode4846
src/ia32/codegen-ia32.cc:4846: answer = EmitNamedGlobalStore(name);
On 2010/02/19 13:11:21, Kevin Millikin wrote:
> I don't really like the name GlobalStore, because not all global stores go
> through it.  EmitContextualStore?
> 
> It's sort of annoying to have a special case for global variables in the code
> generator just to plumb it all the way down to the virtual frame.
> 
> Maybe it would be better to pass an is_contextual flag to EmitNamedStore,
> analogous to EmitNamedLoad.

Done.  is_contextual flag added to EmitNamedStore, and to Frame::CallStoreIC
that it calls.