Thanks Ulan. Comments addressed. PTAL.
https://codereview.chromium.org/191233003/diff/30001/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):
https://codereview.chromium.org/191233003/diff/30001/src/arm/assembler-arm.cc#newcode296
src/arm/assembler-arm.cc:296: // The deserializer needs to know whether
a pointer is specially coded. Bein
On 2014/03/18 12:27:51, ulan wrote:
"Being"
Done.
https://codereview.chromium.org/191233003/diff/30001/src/arm/assembler-arm.cc#newcode1102
src/arm/assembler-arm.cc:1102: // If there is no constant pool
available, we must use an mov immediate.
On 2014/03/18 12:27:51, ulan wrote:
Maybe ASSERT(CpuFeatures::IsSupported(MOVW_MOVT_IMMEDIATE_LOADS))
here?
What we actually want is IsSupported(ARMv7). MOVW_MOVT_IMMEDIATE_LOADS
is only true on qualcomm even to force movw_movt usage (confusing name),
but all armv7 chips support it.
Done.
https://codereview.chromium.org/191233003/diff/30001/src/arm/assembler-arm.cc#newcode3313
src/arm/assembler-arm.cc:3313: if (FLAG_enable_ool_constant_pool)
return;
On 2014/03/18 12:27:51, ulan wrote:
If FLAG_enable_ool_constant_pool then this function should be a no-op,
right?
Maybe assert:
FLAG_enable_ool_constant_pool => num_pending_32_bit_reloc_info_ == 0
FLAG_enable_ool_constant_pool => num_pending_64_bit_reloc_info_ == 0
Done.
https://codereview.chromium.org/191233003/diff/30001/src/arm/assembler-arm.cc#newcode3336
src/arm/assembler-arm.cc:3336: if (FLAG_enable_ool_constant_pool)
return;
On 2014/03/18 12:27:51, ulan wrote:
Same as above.
Done.
https://codereview.chromium.org/191233003/diff/30001/src/arm/assembler-arm.cc#newcode3636
src/arm/assembler-arm.cc:3636: int new_buffer_size = buffer_size_ * 2;
On 2014/03/18 12:27:51, ulan wrote:
Since we already depend on STL, I'd suggest to use std::vector here.
Done.
https://codereview.chromium.org/191233003/diff/30001/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):
https://codereview.chromium.org/191233003/diff/30001/src/arm/builtins-arm.cc#newcode967
src/arm/builtins-arm.cc:967: ConstantPoolUnavailableScope
constant_pool_unavailable(masm);
On 2014/03/18 12:27:51, ulan wrote:
As discussed in one of the previous CLs, let's make the extent of the
scope more
clear by putting it in { }.
Done.
https://codereview.chromium.org/191233003/diff/30001/src/heap.cc
File src/heap.cc (right):
https://codereview.chromium.org/191233003/diff/30001/src/heap.cc#newcode4171
src/heap.cc:4171: maybe_result =
CopyConstantPoolArray(code->constant_pool());
On 2014/03/18 12:27:51, ulan wrote:
Since const pool is smaller then code, it is probably better to
allocate and
copy const pool before allocating and copying the code. I think this
would make
the amount of wasted work smaller if there is an allocation failure.
Done.
https://codereview.chromium.org/191233003/diff/30001/src/heap.cc#newcode4228
src/heap.cc:4228: Object* constant_pool_copy;
On 2014/03/18 12:27:51, ulan wrote:
The same as above.
Done.
https://codereview.chromium.org/191233003/
--
--
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/d/optout.
