Looks good.
I think you should refactor the explicit EmitXXXMapCheck functions, they
won't
scale well.
http://codereview.chromium.org/565034/diff/8001/9008
File src/arm/fast-codegen-arm.cc (right):
http://codereview.chromium.org/565034/diff/8001/9008#newcode64
src/arm/fast-codegen-arm.cc:64: void
FastCodeGenerator::EmitGlobalMapCheck() {
This code is duplicated from EmitReceiverMapCheck. You should define
EmitMapCheck(Register reg, Label* fail, bool is_heap_object) instead of
two separate map check functions.
http://codereview.chromium.org/565034/diff/8001/9008#newcode82
src/arm/fast-codegen-arm.cc:82: PropertyDetails details) {
I'm not a fan of passing this second argument just for the assert.
http://codereview.chromium.org/565034/diff/8001/9007
File src/fast-codegen.cc (right):
http://codereview.chromium.org/565034/diff/8001/9007#newcode217
src/fast-codegen.cc:217: if (!var->is_global()) BAILOUT("Non-global
variable");
I guess this should be !var->is_global() || var->is_this().
For the purposes of simplifying liveness analysis for now, I'm OK with
bailing out for variables we can't handle at compile time.
http://codereview.chromium.org/565034/diff/8001/9004
File src/fast-codegen.h (right):
http://codereview.chromium.org/565034/diff/8001/9004#newcode78
src/fast-codegen.h:78: bool has_receiver() { return
info_->has_receiver(); }
Good to push this function into the CompilationInfo.
http://codereview.chromium.org/565034/diff/8001/9004#newcode84
src/fast-codegen.h:84: bool has_global_object() {
This function should be implemented on CompilationInfo instead.
http://codereview.chromium.org/565034/diff/8001/9004#newcode88
src/fast-codegen.h:88: GlobalObject* global_object() {
And this one.
http://codereview.chromium.org/565034/diff/8001/9004#newcode108
src/fast-codegen.h:108: // global object seen at lazy-compile time.
Just say "compile time". Nothing depends on us using this only for lazy
compilation.
http://codereview.chromium.org/565034/diff/8001/9001
File test/mjsunit/compiler/simple-global-access.js (right):
http://codereview.chromium.org/565034/diff/8001/9001#newcode33
test/mjsunit/compiler/simple-global-access.js:33: function f() { this.x
= this.y = this.z = g1; }
Good. I think we should also check
function g() { this.x = g1; this.y = g2; this.z = g3 }
http://codereview.chromium.org/565034
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
