New version with comments addressed.
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() {
On 2010/02/05 09:48:08, Kevin Millikin wrote:
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.
Done.
http://codereview.chromium.org/565034/diff/8001/9008#newcode82
src/arm/fast-codegen-arm.cc:82: PropertyDetails details) {
On 2010/02/05 09:48:08, Kevin Millikin wrote:
I'm not a fan of passing this second argument just for the assert.
Agree. On the other hand we need to generate different code (checking
for the hole value) when we support deletable properties in the future.
I'll leave it in for now.
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");
On 2010/02/05 09:48:08, Kevin Millikin wrote:
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.
Done.
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(); }
On 2010/02/05 09:48:08, Kevin Millikin wrote:
Good to push this function into the CompilationInfo.
Done.
http://codereview.chromium.org/565034/diff/8001/9004#newcode84
src/fast-codegen.h:84: bool has_global_object() {
On 2010/02/05 09:48:08, Kevin Millikin wrote:
This function should be implemented on CompilationInfo instead.
Done.
http://codereview.chromium.org/565034/diff/8001/9004#newcode88
src/fast-codegen.h:88: GlobalObject* global_object() {
On 2010/02/05 09:48:08, Kevin Millikin wrote:
And this one.
Done.
http://codereview.chromium.org/565034/diff/8001/9004#newcode108
src/fast-codegen.h:108: // global object seen at lazy-compile time.
On 2010/02/05 09:48:08, Kevin Millikin wrote:
Just say "compile time". Nothing depends on us using this only for
lazy
compilation.
Done.
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; }
On 2010/02/05 09:48:08, Kevin Millikin wrote:
Good. I think we should also check
function g() { this.x = g1; this.y = g2; this.z = g3 }
Done.
http://codereview.chromium.org/565034
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
