Hi Hannes, see my comments re: a unit test and a minor code comment. This is
great stuff!
--Michael
https://codereview.chromium.org/12314155/diff/12001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/12314155/diff/12001/src/hydrogen.cc#newcode1064
src/hydrogen.cc:1064: }
This is a cool example of how you request in hydrogen an old space
allocation. This place is allocating a FixedArray, I think as part of a
JSArray? I think it might be good to also have the "parent" JSArray
object be in the old space too.
Also, there is no intelligence behind the pretenuring, yet. Is this just
to show that if you turn on the flag the mechanism works?
In that case maybe build a little extra stuff. What about
1) A runtime function to ask:
SPACE GetObjectSpace(obj)
2) a unit test that uses this to show that the old space allocations
work. You'd turn on the new flag for that test, otherwise it'll continue
being off by default.
--> Soon enough there'll be all kinds of neat rules behind the
pre-tenuring decision, and the complexity of this simple unit test will
grow nicely.
https://codereview.chromium.org/12314155/diff/12001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
https://codereview.chromium.org/12314155/diff/12001/src/ia32/lithium-codegen-ia32.cc#newcode5519
src/ia32/lithium-codegen-ia32.cc:5519:
MacroAssembler::OLD_POINTER_SPACE);
Maybe it's nice to have a variable for the space to allocate in, setting
it via ternary operator like
Space space = instr->hydrogen()->CanAllocateInOldPointerSpace()
? MacroAssembler::OLD_POINTER_SPACE
: MacroAssembler::NEW_SPACE;
then you just need one __ Allocate() call.
https://codereview.chromium.org/12314155/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
