On 2014/09/26 15:54:58, rmcilroy wrote:
https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h
File src/ia32/assembler-ia32.h (right):
https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h#newcode1102
src/ia32/assembler-ia32.h:1102: // Support not implemented in ia32 yet.
We will probably never us a constant pool on ia32, so you can just remove
the
comment and UNREACHABLE and always return false here.
https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h#newcode1115
src/ia32/assembler-ia32.h:1115: bool is_const_pool_blocked() const {
You don't need this function (that is only for the inline constant pool
and
will
be going away on Arm once we switch to OOL constant pool).
https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h#newcode1122
src/ia32/assembler-ia32.h:1122: // Support not implemented in ia32 yet.
replace comment with: "// Constant pool is not supported on ia32"
https://codereview.chromium.org/609843002/diff/1/src/ia32/macro-assembler-ia32.cc
File src/ia32/macro-assembler-ia32.cc (right):
https://codereview.chromium.org/609843002/diff/1/src/ia32/macro-assembler-ia32.cc#newcode902
src/ia32/macro-assembler-ia32.cc:902: bool load_constant_pool) {
Add an ASSERT that load_constant_pool is always false. Also could you
rename
the variable (both here and on arm) to load_constant_pool_pointer_reg
please.
https://codereview.chromium.org/609843002/diff/1/src/macro-assembler.h
File src/macro-assembler.h (right):
https://codereview.chromium.org/609843002/diff/1/src/macro-assembler.h#newcode97
src/macro-assembler.h:97: bool enter_frame = true)
I don't like this default argument. Rethinking this I think we should
avoid
making FrameAndConstantPoolScope a subclass of FrameScope. It doesn't
buy us
much and there are subtle ordering constraints (like
set_constant_pool_available
having to happen before masm->EnterFrame and after LeaveFrame) which are
difficult to ensure with this class inheritance.
Could you also make the description and CL title more descriptive please
(mentioning this is re-factoring to enable it to be used by other
architectures).
https://codereview.chromium.org/609843002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
