http://codereview.chromium.org/3141022/diff/28006/34008 File src/arm/macro-assembler-arm.cc (right):
http://codereview.chromium.org/3141022/diff/28006/34008#newcode1340 src/arm/macro-assembler-arm.cc:1340: void MacroAssembler::IndexFromHash(Register key, Register hash) { On 2010/08/27 08:57:59, Vyacheslav Egorov wrote:
makes sense to rename key to index and reorder arguments so that out
reg goes
last.
Done. http://codereview.chromium.org/3141022/diff/28006/34008#newcode1343 src/arm/macro-assembler-arm.cc:1343: // hash - holds the key's hash. Clobbered. On 2010/08/27 08:57:59, Vyacheslav Egorov wrote:
Worth moving this comment to header file. It is important to state
that hash
will be clobbered.
Done. http://codereview.chromium.org/3141022/diff/28006/34008#newcode1355 src/arm/macro-assembler-arm.cc:1355: // there is no difference in using either key. On 2010/08/27 08:57:59, Vyacheslav Egorov wrote:
this comment about clobbering makes no sense after this function
became public
and generic.
The comment removed. http://codereview.chromium.org/3141022/diff/28006/34008#newcode1356 src/arm/macro-assembler-arm.cc:1356: STATIC_ASSERT(String::kHashShift >= kSmiTagSize); On 2010/08/27 08:57:59, Vyacheslav Egorov wrote:
This assert makes no sense (we do not use expression kHashShift -
kSmiTagSize)
Better assert kSmiTag == 0
The comment replaced. http://codereview.chromium.org/3141022/diff/28006/34015 File src/ia32/macro-assembler-ia32.cc (right): http://codereview.chromium.org/3141022/diff/28006/34015#newcode1055 src/ia32/macro-assembler-ia32.cc:1055: // Register use: On 2010/08/27 08:57:59, Vyacheslav Egorov wrote:
see comments from arm macroassembler, all of them [except for comment
about
String::kHashShift >= kSmiTagSize assert] apply to this method.
Done. http://codereview.chromium.org/3141022/diff/28006/34017 File src/runtime.cc (right): http://codereview.chromium.org/3141022/diff/28006/34017#newcode4864 src/runtime.cc:4864: ASSERT_EQ((subject->Hash(), static_cast<int>(subject->hash_field())), On 2010/08/27 08:57:59, Vyacheslav Egorov wrote:
This usage of comma-expression looks a bit hackerish to me. Can we
avoid it? Comma-expression well defined in the C++ Standard so I don't think i's hackerish. However I moved subject->Hash() to a separate statement in "#ifdef DEBUG" block. http://codereview.chromium.org/3141022/diff/28006/34024 File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/3141022/diff/28006/34024#newcode395 src/x64/macro-assembler-x64.cc:395: // Register use: On 2010/08/27 08:57:59, Vyacheslav Egorov wrote:
comments are same as for ia32 macroassembler
Done. http://codereview.chromium.org/3141022/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
