LGTM with tiny nits.

https://chromiumcodereview.appspot.com/9496010/diff/1/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):

https://chromiumcodereview.appspot.com/9496010/diff/1/src/arm/stub-cache-arm.cc#newcode199
src/arm/stub-cache-arm.cc:199: // entry size being 12.
Hmmm, I can see multiplication by 3 and a left shift by one, but that
leaves a factor of 2. To be honest, I don't fully understand the
calculations in ProbeTable, some comments in prose would be helpful...

https://chromiumcodereview.appspot.com/9496010/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://chromiumcodereview.appspot.com/9496010/diff/1/src/flag-definitions.h#newcode566
src/flag-definitions.h:566: DEFINE_bool(test_secondary_stub_cache,
These should probably be debug-only flags.

https://chromiumcodereview.appspot.com/9496010/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://chromiumcodereview.appspot.com/9496010/diff/1/test/cctest/test-api.cc#newcode16112
test/cctest/test-api.cc:16112: static int* LookupCounter(const char*
name) {
It seems that all those statics here are only used in DEBUG mode, so I
guess that gcc will complain in release mode.

https://chromiumcodereview.appspot.com/9496010/diff/1/test/cctest/test-api.cc#newcode16138
test/cctest/test-api.cc:16138: TEST(SecondaryStubCache) {
This differs from TEST(PrimaryStubCache) only in a single line, so a
separate method for the common code might be appropriate.

Just out of curiosity: Where/how do we set/reset the i::FLAG_foo flags
for each test?

https://chromiumcodereview.appspot.com/9496010/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to