Thanks for the patch. I like this cleanup a lot, perhaps you can just go a
little further (see comments).
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);
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.
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);
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);
See comment above about using constants.
https://codereview.chromium.org/22267005/diff/12001/src/x64/code-stubs-x64.cc#newcode4978
src/x64/code-stubs-x64.cc:4978: __ ret(3 * kPointerSize);
You can use the argument count instead of a magic number here and below
of if you define the enum.
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);
Why not (StackHandlerConstants::kSiz == 4 * kPointerSize +
kFPSizeOnStack)? Or do I misunderstand something? The >= comparison
generally seems a lot weaker (same below)
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));
1 instead of argc
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.
