Addressed comments + Merged with Kevin's change.
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() {
On 2010/02/03 14:22:20, Kevin Millikin wrote:
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.
I'm not sure about the map check vs. identity check. Can there be a map
transition on the global object? In that case I have to check the map,
right?
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());
On 2010/02/03 14:22:20, Kevin Millikin wrote:
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.
Done.
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,
On 2010/02/03 14:22:20, Kevin Millikin wrote:
I think you actually want the JSFunction being compiled here. I have
an
outstanding change to add it to the CompilationInfo.
Done.
http://codereview.chromium.org/565034/diff/4001/4013#newcode50
src/compiler.h:50: has_global_var_access_(false) {
On 2010/02/03 14:22:20, Kevin Millikin wrote:
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_.
Done.
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()) {
On 2010/02/03 14:22:20, Kevin Millikin wrote:
var->is_global() && !var->is_this()?
Done.
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());
On 2010/02/03 14:22:20, Kevin Millikin wrote:
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));
...
}
Good point. I remember trying using Lookup - for some reasone I did it
this way. Let me check again...
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);
On 2010/02/03 14:22:20, Kevin Millikin wrote:
"FromCell" is implicit in the argument.
Done.
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,
On 2010/02/03 14:22:20, Kevin Millikin wrote:
As per my other comments, I think you want to get the global object
from the
JSFunction.
Done.
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;
On 2010/02/03 14:22:20, Kevin Millikin wrote:
This is a bad code smell. I don't think the CallIC class should need
the code
generator even as a utility.
Done. The IC code is left completely unchanged.
http://codereview.chromium.org/565034
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
