LGTM with nits.
https://codereview.chromium.org/530783002/diff/80001/src/compiler/arm/linkage-arm.cc
File src/compiler/arm/linkage-arm.cc (right):
https://codereview.chromium.org/530783002/diff/80001/src/compiler/arm/linkage-arm.cc#newcode17
src/compiler/arm/linkage-arm.cc:17: struct ArmLinkageHelperTraits {
I think this struct and the typedef below should be in an anonymous
namespace. Then you don't need to prefix with architecture name. (Same
for other linkage-arch files).
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage-impl.h
File src/compiler/linkage-impl.h (right):
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage-impl.h#newcode29
src/compiler/linkage-impl.h:29: int js_parameter_count) {
size_t
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage-impl.h#newcode44
src/compiler/linkage-impl.h:44: for (int i = 0; i < js_parameter_count;
i++) {
size_t
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage-impl.h#newcode71
src/compiler/linkage-impl.h:71: Zone* zone, Runtime::FunctionId
function_id, int js_parameter_count,
size_t
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage.h
File src/compiler/linkage.h (right):
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage.h#newcode68
src/compiler/linkage.h:68: js_param_count_(js_param_count),
Nit: Rename to js_parameter_count_ for consistency.
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage.h#newcode109
src/compiler/linkage.h:109: MachineSignature* GetMachineSignature()
const { return machine_sig_; }
Nit: const MachineSignature*
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage.h#newcode136
src/compiler/linkage.h:136: size_t js_param_count_;
Nit: js_parameter_count_
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage.h#newcode202
src/compiler/linkage.h:202: MachineType GetParameterType(int index) {
Nit: method should be const
https://codereview.chromium.org/530783002/diff/80001/src/compiler/linkage.h#newcode212
src/compiler/linkage.h:212: MachineType GetReturnType() { return
incoming_->GetReturnType(0); }
Nit: method should be const
https://codereview.chromium.org/530783002/diff/80001/src/compiler/x64/linkage-x64.cc
File src/compiler/x64/linkage-x64.cc (right):
https://codereview.chromium.org/530783002/diff/80001/src/compiler/x64/linkage-x64.cc#newcode38
src/compiler/x64/linkage-x64.cc:38: static Register
CRegisterParameter(int i) {
This parameter should be size_t everywhere.
(Did I mention that I really hate this Linkage templatization?)
https://codereview.chromium.org/530783002/diff/80001/src/compiler/x64/linkage-x64.cc#newcode47
src/compiler/x64/linkage-x64.cc:47: static int
CRegisterParametersLength() { return kWin64 ? 4 : 6; }
This should return size_t as well.
https://codereview.chromium.org/530783002/
--
--
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/d/optout.
