Florian,
thanks a lot for your comments.
http://codereview.chromium.org/5753005/diff/1/src/compiler.cc
File src/compiler.cc (right):
http://codereview.chromium.org/5753005/diff/1/src/compiler.cc#newcode168
src/compiler.cc:168: if (!info->AllowOptimize())
info->DisableOptimization();
On 2010/12/13 19:00:18, fschneider wrote:
This checks if the function literal's scope has any heap allocated
slots. So I
think this check can go away with your change.
I am not sure: CompilationInfo::AllowOptimize checks if crankshaft is
enabled and if closure_ is not null. I removed Scope::AllowOptimize as
per your suggestion on IM---thanks a lot for spotting this.
http://codereview.chromium.org/5753005/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
http://codereview.chromium.org/5753005/diff/1/src/hydrogen-instructions.h#newcode2520
src/hydrogen-instructions.h:2520: virtual Representation
RequiredInputRepresentation(int index) const {
On 2010/12/13 19:00:18, fschneider wrote:
Since HLoadContextSlot does not have any input operands, you do not
need to
implement RequiredInputRepresentation.
Done
http://codereview.chromium.org/5753005/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/5753005/diff/1/src/hydrogen.cc#newcode2372
src/hydrogen.cc:2372: if (scope->num_heap_slots() > 0) BAILOUT("heap
allocated locals");
On 2010/12/13 19:00:18, fschneider wrote:
I think this bailout can go away, too.
I am not sure. Apparently it makes us bail out of the function being
compiled needs to store some local variables in heap allocated slots,
not when the function accesses heap allocated slot of some other
function. That is:
function foo() {
var heap_slot = 1;
return function bar() {
return heap_slot;
}
}
This check bails out for foo, not for bar, if I understand it correctly.
http://codereview.chromium.org/5753005/diff/1/src/hydrogen.cc#newcode2934
src/hydrogen.cc:2934: variable->AsSlot()->type() == Slot::CONTEXT) {
On 2010/12/13 19:00:18, fschneider wrote:
Indentation.
Done.
http://codereview.chromium.org/5753005/diff/1/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):
http://codereview.chromium.org/5753005/diff/1/src/ia32/lithium-ia32.h#newcode1287
src/ia32/lithium-ia32.h:1287: public:
On 2010/12/13 19:00:18, fschneider wrote:
Please also implement the PrintDataTo function.
Done.
http://codereview.chromium.org/5753005/diff/1/src/objects-inl.h
File src/objects-inl.h (right):
http://codereview.chromium.org/5753005/diff/1/src/objects-inl.h#newcode2987
src/objects-inl.h:2987: void SharedFunctionInfo::set_code(Code* value,
WriteBarrierMode mode) {
On 2010/12/13 19:00:18, fschneider wrote:
I'm not sure why is this assert not needed anymore? (I'm also not sure
why is
was needed before)
My reasoning: before only functions which allowed lazy compilation were
optimizable, now any function has a chance, hence the assert should go
away. Does that sound reasonable?
http://codereview.chromium.org/5753005/diff/1/test/cctest/cctest.status
File test/cctest/cctest.status (right):
http://codereview.chromium.org/5753005/diff/1/test/cctest/cctest.status#newcode32
test/cctest/cctest.status:32: # The problem is code object can get
different optimizable flag in crankshaft after creation.
On 2010/12/13 19:00:18, fschneider wrote:
# The problem is that a code object can get a different
# optimizable flag in crankshaft after creation.
Done.
http://codereview.chromium.org/5753005/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
