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.
On 2014/09/26 15:54:57, rmcilroy wrote:
We will probably never us a constant pool on ia32, so you can just
remove the
comment and UNREACHABLE and always return false here.

Done.

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 {
On 2014/09/26 15:54:57, rmcilroy wrote:
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).

Done.

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.
On 2014/09/26 15:54:57, rmcilroy wrote:
replace comment with: "// Constant pool is not supported on ia32"

Done.

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) {
On 2014/09/26 15:54:57, rmcilroy wrote:
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.

Done.

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)
On 2014/09/26 15:54:57, rmcilroy wrote:
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.

I was worried about code duplication, since this class is very similar
to FrameScope. But yes, the subclass approach will only work if the
ordering changes are not an issue. I reverted back to the original
FrameAndConstantPoolScope class.

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.

Reply via email to