This already looks quite good.
Can you rebase it on bleeding_edge? I refactored HashMap, which should
simplify
this CL a bit.
http://codereview.chromium.org/9455088/diff/46/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/9455088/diff/46/src/arm/code-stubs-arm.cc#newcode7056
src/arm/code-stubs-arm.cc:7056: { REG(R6), REG(R4), REG(R7),
EMIT_REMEMBERED_SET },
It would be nice to use the same names here as well like r6, r4, r7.
This way they are easier to find when searching for all occurrences of a
register.
Maybe another macro like
DECLARE_STUB(r6, r4, r7, EMIT_REMEMBERED_SET) would help.
http://codereview.chromium.org/9455088/diff/46/src/hashmap.h
File src/hashmap.h (right):
http://codereview.chromium.org/9455088/diff/46/src/hashmap.h#newcode47
src/hashmap.h:47: class HashMap {
I changed HashMap so that it does not have an allocator_ member, so I
think hashmap.h and hashmap.cc should be fine now.
http://codereview.chromium.org/9455088/diff/46/src/lithium-allocator.cc
File src/lithium-allocator.cc (right):
http://codereview.chromium.org/9455088/diff/46/src/lithium-allocator.cc#newcode56
src/lithium-allocator.cc:56: &name::cache);
\
I'd move this initialization to lithium.cc since it rather belongs to
where LOperand is defined.
http://codereview.chromium.org/9455088/diff/46/src/lithium.h
File src/lithium.h (left):
http://codereview.chromium.org/9455088/diff/46/src/lithium.h#oldcode248
src/lithium.h:248:
Accidental edit?
http://codereview.chromium.org/9455088/diff/46/src/lithium.h
File src/lithium.h (right):
http://codereview.chromium.org/9455088/diff/46/src/lithium.h#newcode263
src/lithium.h:263: SetUpCache();
Creation of LOperand is a really frequent operation. The call to
SetupCache should maybe moved to global V8 initialization in
InitializeOncePerProcessImpl.
http://codereview.chromium.org/9455088/diff/46/src/scopes.cc
File src/scopes.cc (right):
http://codereview.chromium.org/9455088/diff/46/src/scopes.cc#newcode88
src/scopes.cc:88: VariableMap::VariableMap() : HashMap(Match,
LocalsMapAllocator.Pointer(), 8) {}
Not needed anymore because HashMap is already refactored to avoid a
static initializer.
http://codereview.chromium.org/9455088/diff/46/tools/check-static-initializers.sh
File tools/check-static-initializers.sh (right):
http://codereview.chromium.org/9455088/diff/46/tools/check-static-initializers.sh#newcode36
tools/check-static-initializers.sh:36: v8_out=$(readlink -f $(dirname
$BASH_SOURCE)/../out)
Not sure if this works with the current build configuration on the
buildbot which I believe uses the SCons build. Somehow specifying the
binary output directory would be useful, I guess.
http://codereview.chromium.org/9455088/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
