danno, thanks for the review. All the comments are addressed.
I will rebase with master to handle
https://codereview.chromium.org/23903053 in
this CL.
https://codereview.chromium.org/22267005/diff/12001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):
https://codereview.chromium.org/22267005/diff/12001/src/x64/code-stubs-x64.cc#newcode2396
src/x64/code-stubs-x64.cc:2396: StackArgumentsAccessor args(rsp, 4,
ARGUMENTS_DONT_CONTAIN_RECEIVER);
On 2013/09/18 12:08:42, danno wrote:
Since you are going through all the trouble to clean this up, you also
please
turn the hard coded numbers into constants?
enum RegExpExecStubArgumentIndices {
JS_REG_EXP_OBJECT_ARGUMENT_INDEX,
SUBJECT_STRING_ARGUMENT_INDEX,
PREVIOUS_INDEX_ARGUMENT_INDEX,
LAST_MATCH_INFO_ARGUMENT_INDEX,
REG_EXP_EXEC_ARGUMENT_COUNT
}
And use them where appropriate?
This also applies to the changes later in this file.
Done.
https://codereview.chromium.org/22267005/diff/12001/src/x64/code-stubs-x64.cc#newcode2397
src/x64/code-stubs-x64.cc:2397: Operand StackOperandForJSRegExpObject =
args.GetArgumentOperand(0);
Removed these variables.
On 2013/09/18 12:08:42, danno wrote:
nit: these variable names should start with a lower case letter.
https://codereview.chromium.org/22267005/diff/12001/src/x64/code-stubs-x64.cc#newcode4951
src/x64/code-stubs-x64.cc:4951: StackArgumentsAccessor args(rsp, 3,
ARGUMENTS_DONT_CONTAIN_RECEIVER);
On 2013/09/18 12:08:42, danno wrote:
See comment above about using constants.
Done.
https://codereview.chromium.org/22267005/diff/12001/src/x64/code-stubs-x64.cc#newcode4978
src/x64/code-stubs-x64.cc:4978: __ ret(3 * kPointerSize);
On 2013/09/18 12:08:42, danno wrote:
You can use the argument count instead of a magic number here and
below of if
you define the enum.
Done.
https://codereview.chromium.org/22267005/diff/12001/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):
https://codereview.chromium.org/22267005/diff/12001/src/x64/macro-assembler-x64.cc#newcode2653
src/x64/macro-assembler-x64.cc:2653:
STATIC_ASSERT(StackHandlerConstants::kSize >= 5 * kPointerSize);
On 2013/09/18 12:08:42, danno wrote:
Why not (StackHandlerConstants::kSiz == 4 * kPointerSize +
kFPSizeOnStack)? Or
do I misunderstand something? The >= comparison generally seems a lot
weaker
(same below)
Done.
https://codereview.chromium.org/22267005/diff/12001/src/x64/stub-cache-x64.cc
File src/x64/stub-cache-x64.cc (right):
https://codereview.chromium.org/22267005/diff/12001/src/x64/stub-cache-x64.cc#newcode2336
src/x64/stub-cache-x64.cc:2336: __ movq(rax,
args.GetArgumentOperand(argc));
On 2013/09/18 12:08:42, danno wrote:
1 instead of argc
Done.
https://codereview.chromium.org/22267005/
--
--
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/groups/opt_out.
