I don't think the global object of the caller is the one you want to
specialize
for. I think you should be able to make this change without changing the
call
ICs at all.
There is some overlap with my change to add the closure to the
CompilationInfo
structure which was done exactly to enable specialization for global
load/stores.
http://codereview.chromium.org/565034/diff/4001/4020
File src/arm/fast-codegen-arm.cc (right):
http://codereview.chromium.org/565034/diff/4001/4020#newcode64
src/arm/fast-codegen-arm.cc:64: void
FastCodeGenerator::EmitGlobalMapCheck() {
Why check the map of the global object and not the identity of the
object itself?
If you need the map check, combine the nearly identical code from
EmitGlobalMapCheck and EmitReceiverMapCheck. You can pass a flag
indicating whether you know staticallyu that it's a heap object.
http://codereview.chromium.org/565034/diff/4001/4022
File src/arm/ic-arm.cc (right):
http://codereview.chromium.org/565034/diff/4001/4022#newcode388
src/arm/ic-arm.cc:388: __ ldr(r0, CodeGenerator::GlobalObject());
This seems weird. The CallIC miss calls CompileLazy or
CompileLazyInLoop with the function to compile. The function has a
context. The context has a global object. Isn't that the one you want
to specialize for?
The global object of the caller seems wrong.
http://codereview.chromium.org/565034/diff/4001/4013
File src/compiler.h (right):
http://codereview.chromium.org/565034/diff/4001/4013#newcode43
src/compiler.h:43: Handle<Object> global_object,
I think you actually want the JSFunction being compiled here. I have an
outstanding change to add it to the CompilationInfo.
http://codereview.chromium.org/565034/diff/4001/4013#newcode50
src/compiler.h:50: has_global_var_access_(false) {
If you change one, please change the name of all the has_XXX_ functions
to keep them uniform. has_this_properties_ ==>
has_this_property_access_.
http://codereview.chromium.org/565034/diff/4001/4005
File src/data-flow.cc (right):
http://codereview.chromium.org/565034/diff/4001/4005#newcode166
src/data-flow.cc:166: if (var->is_global()) {
var->is_global() && !var->is_this()?
http://codereview.chromium.org/565034/diff/4001/4017
File src/fast-codegen.cc (right):
http://codereview.chromium.org/565034/diff/4001/4017#newcode495
src/fast-codegen.cc:495: Handle<GlobalObject> object =
Handle<GlobalObject>::cast(global_object());
This seems too low level. Can't you do something like:
LookupResult lookup;
global->Lookup(*expr->name(), &lookup);
if (lookup.GetPropertyDetails().IsDontDelete()) {
...
Handle<Object> cell(global->GetPropertyCell(&lookup));
...
}
http://codereview.chromium.org/565034/diff/4001/4006
File src/fast-codegen.h (right):
http://codereview.chromium.org/565034/diff/4001/4006#newcode116
src/fast-codegen.h:116: void
EmitGlobalVariableLoadFromCell(Handle<Object> cell);
"FromCell" is implicit in the argument.
http://codereview.chromium.org/565034/diff/4001/4018
File src/handles.h (right):
http://codereview.chromium.org/565034/diff/4001/4018#newcode324
src/handles.h:324: Handle<Object> global_object,
As per my other comments, I think you want to get the global object from
the JSFunction.
http://codereview.chromium.org/565034/diff/4001/4008
File src/ia32/codegen-ia32.h (right):
http://codereview.chromium.org/565034/diff/4001/4008#newcode648
src/ia32/codegen-ia32.h:648: friend class CallIC;
This is a bad code smell. I don't think the CallIC class should need
the code generator even as a utility.
http://codereview.chromium.org/565034
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
