https://codereview.chromium.org/1131783003/diff/40001/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):
https://codereview.chromium.org/1131783003/diff/40001/src/arm/assembler-arm.cc#newcode478
src/arm/assembler-arm.cc:478: offset = EmitEmbeddedConstantPool();
On 2015/06/01 09:52:08, rmcilroy wrote:
/s/offset/constant_pool_offset
Done.
https://codereview.chromium.org/1131783003/diff/40001/src/assembler.h
File src/assembler.h (right):
https://codereview.chromium.org/1131783003/diff/40001/src/assembler.h#newcode1154
src/assembler.h:1154: // Embedded constant pool support
On 2015/06/01 09:52:08, rmcilroy wrote:
ConstntPoolEntry is used by both embedded and inline constant pools
now right?
Change the comment here to "Constant pool support" and add one above
ConstantPoolBuilder with "Embedded constant pool support"
Done.
https://codereview.chromium.org/1131783003/diff/40001/src/assembler.h#newcode1189
src/assembler.h:1189: friend class ConstantPoolBuilder;
On 2015/06/01 09:52:09, rmcilroy wrote:
is this only for setting merged_index_? If so, I would prefer to add a
set_merged_index() function rather than requiring that the classes be
friended
Done.
https://codereview.chromium.org/1131783003/diff/40001/src/ppc/lithium-codegen-ppc.cc
File src/ppc/lithium-codegen-ppc.cc (right):
https://codereview.chromium.org/1131783003/diff/40001/src/ppc/lithium-codegen-ppc.cc#newcode57
src/ppc/lithium-codegen-ppc.cc:57: // Avoid DCHECK(!is_linked()) failure
in ~Label()
On 2015/06/01 09:52:09, rmcilroy wrote:
Won't this cause issues in non-debug code if the label is getting
deleted before
the constant pool is emitted?
Not quite. The label gets deleted upon destruction of the assembler
object. The issue here is that the embedded constant pool label may be
linked to in the prologue, but never bound since this path aborts the
compilation prior to reaching GetCode (where the constant pool would
have been emitted).
If there is a better way to work around this, let me know. It seems
like the labels used for deferred code generation could also run into
this. Perhaps the tests just don't exercise that scenario?
https://codereview.chromium.org/1131783003/diff/40001/test/cctest/test-constantpool.cc
File test/cctest/test-constantpool.cc (right):
https://codereview.chromium.org/1131783003/diff/40001/test/cctest/test-constantpool.cc#newcode1
test/cctest/test-constantpool.cc:1: // Copyright 2013 the V8 project
authors. All rights reserved.
On 2015/06/01 09:52:09, rmcilroy wrote:
2015. Also you just need the short copyright notice now:
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can
be
// found in the LICENSE file.
https://www.chromium.org/developers/coding-style#TOC-File-headers
Done.
https://codereview.chromium.org/1131783003/diff/40001/test/cctest/test-constantpool.cc#newcode43
test/cctest/test-constantpool.cc:43: const int kReach = 1 << kReachBits;
On 2015/06/01 09:52:09, rmcilroy wrote:
nit - could you add one test which has dblReach different from
intReach to check
that works as intended.
Done.
https://codereview.chromium.org/1131783003/diff/40001/test/cctest/test-constantpool.cc#newcode218
test/cctest/test-constantpool.cc:218: }
On 2015/06/01 09:52:09, rmcilroy wrote:
It would be nice if you could test the emitting logic as well, though
I realize
this would be tricky to do in a non-arch specific way. Would it be
possible to
add some test of the emitting logic to test-assembler-ppc?
I considered this, however other than validating the expected size of
the emitted pool, there isn't much else to assert on that adds value
beyond just running the rest of the tests with embedded pools enabled.
I'd like to consider this further as a separate CL. PPC in general needs
better coverage in test-assembler-ppc.
https://codereview.chromium.org/1131783003/
--
--
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.
