Increase coverage of global loads in optimized code...

Increase coverage of global loads in optimized code

In the cases where a global property cell cannot be used in the optimized code use standard load ic to get the property instead of bailing out.

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

[Søren Thygesen Gjesse, 2011-03-11 13:36:50]

If this looks good I will port it to the other platforms.

I will do the same for store in a separate change.

[fschneider, 2011-03-11 13:52:04]

I can review, since I have an almost identical cl.

[Søren Thygesen Gjesse, 2011-03-16 08:22:03]

I have now refactored to distinguish better between the load from a cell and
using the generic IC and ported to all platforms.

Loads in a typeof context is also handled correctly.

When using a generic load never inline calls to functions retrieved that way, as
the bailout for when the inlined function changes has no corresponding point in
the full code.

Please scrutinize the added PrepareForBailoutForId in the full compiler for
correctness.

[fschneider, 2011-03-16 10:28:18]

LGTM.

http://codereview.chromium.org/6665026/diff/7002/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/6665026/diff/7002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:3808: PrepareForBailout(expr->increment(), TOS_REG);
I think it does not hurt here, but shouldn't this bailout be prepared only for
named and keyed properties then?

http://codereview.chromium.org/6665026/diff/7002/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/6665026/diff/7002/src/hydrogen.cc#newcode2814
src/hydrogen.cc:2814: return kUseGeneric;
I'd move this into the same line as the if-statement, if it fits. (also in the
ones below)

http://codereview.chromium.org/6665026/diff/7002/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6665026/diff/7002/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:2038: __ mov(eax, Operand(esi,
Context::SlotOffset(Context::GLOBAL_INDEX)));
You could consider making LoadGlobalGeneric take two iput operands: the context
in esi and the global object in eax, so you don't need to reload it here. In the
hydrogen IR there exists already a HGlobalObject instruction.

[Søren Thygesen Gjesse, 2011-03-17 07:52:27]

http://codereview.chromium.org/6665026/diff/7002/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/6665026/diff/7002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:3808: PrepareForBailout(expr->increment(), TOS_REG);
On 2011/03/16 10:28:18, fschneider wrote:
> I think it does not hurt here, but shouldn't this bailout be prepared only for
> named and keyed properties then?

That looks right, done (on all platforms).

http://codereview.chromium.org/6665026/diff/7002/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/6665026/diff/7002/src/hydrogen.cc#newcode2814
src/hydrogen.cc:2814: return kUseGeneric;
On 2011/03/16 10:28:18, fschneider wrote:
> I'd move this into the same line as the if-statement, if it fits. (also in the
> ones below)

Done.

http://codereview.chromium.org/6665026/diff/7002/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6665026/diff/7002/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:2038: __ mov(eax, Operand(esi,
Context::SlotOffset(Context::GLOBAL_INDEX)));
On 2011/03/16 10:28:18, fschneider wrote:
> You could consider making LoadGlobalGeneric take two iput operands: the
context
> in esi and the global object in eax, so you don't need to reload it here. In
the
> hydrogen IR there exists already a HGlobalObject instruction.

Done.