https://codereview.chromium.org/384403002/diff/130001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/384403002/diff/130001/src/code-stubs-hydrogen.cc#newcode42
src/code-stubs-hydrogen.cc:42: int parameter_count =
descriptor_->register_param_count() - 1;
GetEnvironmentParameterCount
https://codereview.chromium.org/384403002/diff/130001/src/code-stubs-hydrogen.cc#newcode122
src/code-stubs-hydrogen.cc:122: int param_count =
descriptor_->register_param_count() - 1;
Use GetEnvironmentLength(), then comment is unnecessary
https://codereview.chromium.org/384403002/diff/130001/src/code-stubs-hydrogen.cc#newcode132
src/code-stubs-hydrogen.cc:132: int parameter_index = i+1;
nit: make sure to clang format your patch. Comment about accounting for
the context parameter?
https://codereview.chromium.org/384403002/diff/130001/src/code-stubs.h
File src/code-stubs.h (right):
https://codereview.chromium.org/384403002/diff/130001/src/code-stubs.h#newcode276
src/code-stubs.h:276: bool initialized() const { return
register_param_count_ >= 0; }
style: should be IsInitialized()
https://codereview.chromium.org/384403002/diff/130001/src/code-stubs.h#newcode280
src/code-stubs.h:280: }
I think with a little more "duplication", you can make this clearer. In
addition to GetEnvironmentLength, you should have
GetEnvironmentParameterCount (which should actually be
register_param_count_ - 1), which is used really where you current -1
due to the smaller environment, and there you shouldn't have to do the
math.
https://codereview.chromium.org/384403002/diff/130001/src/code-stubs.h#newcode282
src/code-stubs.h:282: int register_param_count() const { return
register_param_count_; }
For consistency, make this GetRegisterParameterCount()
https://codereview.chromium.org/384403002/diff/130001/src/deoptimizer.cc
File src/deoptimizer.cc (right):
https://codereview.chromium.org/384403002/diff/130001/src/deoptimizer.cc#newcode1559
src/deoptimizer.cc:1559: int param_count =
descriptor->register_param_count() - 1;
Use GetEnvironmentParameterLength(), then comment is unnecessary
https://codereview.chromium.org/384403002/diff/130001/src/deoptimizer.cc#newcode1711
src/deoptimizer.cc:1711: // the descriptor parameter array accordingly.
Why not move this up to the loop iteration? e.g.
for (int i = 1; i < param_count; ++i) {
Otherwise, I think you might actually do one too many iterations here,
since you access param_count + 1, which is technically OOB
https://codereview.chromium.org/384403002/diff/130001/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):
https://codereview.chromium.org/384403002/diff/130001/src/ia32/lithium-ia32.h#newcode1869
src/ia32/lithium-ia32.h:1869: :
inputs_(descriptor->GetEnvironmentLength() + 1, zone) {
This (and below and on other platforms should get
GetRegisterParameterCount() + 1
https://codereview.chromium.org/384403002/
--
--
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.
