Overall the change lgtm. Could you upload a rebased version?
It would be see how much it changes the size of our relocation info overall.
I'd consider not changing the type of ast ids and leave them as integers,
since
it would make the change a lot smaller without changing the semantics.
http://codereview.chromium.org/5699002/diff/3001/src/assembler.cc
File src/assembler.cc (left):
http://codereview.chromium.org/5699002/diff/3001/src/assembler.cc#oldcode92
src/assembler.cc:92: // The encoding relies on the fact that there are
less than 14
I'm confused by this comment in the existing code since the ASSERT below
says <= 14
http://codereview.chromium.org/5699002/diff/3001/src/assembler.cc
File src/assembler.cc (right):
http://codereview.chromium.org/5699002/diff/3001/src/assembler.cc#newcode97
src/assembler.cc:97: // code_taget: [6 bits pc delta] 01
--> code_target
http://codereview.chromium.org/5699002/diff/3001/src/assembler.cc#newcode99
src/assembler.cc:99: // code_taget_with_id: [6 bits pc delta] 10,
--> code_target
http://codereview.chromium.org/5699002/diff/3001/src/assembler.h
File src/assembler.h (right):
http://codereview.chromium.org/5699002/diff/3001/src/assembler.h#newcode202
src/assembler.h:202: NUMBER_OF_MODES, // must be no greater than ?? -
see RelocInfoWriter
Update comment. Maybe a STATIC_ASSERT would be better than just a
comment or normal ASSERT.
http://codereview.chromium.org/5699002/diff/3001/src/v8globals.h
File src/v8globals.h (right):
http://codereview.chromium.org/5699002/diff/3001/src/v8globals.h#newcode108
src/v8globals.h:108: typedef unsigned AstId;
Maybe consider leaving ast id as integers (like before) - although I
like having a separate type AstId, I'm not sure if the typedef alone
gives us much benefit overall.
It would also affect less source lines if we leave the type for ast ids
unchanged (i.e. making it easier to merge).
http://codereview.chromium.org/5699002/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
