Wow, this is quite heavy refactoring. But I really like the direction this
is
going, will make it a lot easier to maintain/change the deoptimizer in the
future. First round of comments below.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.cc
File src/deoptimizer.cc (right):
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.cc#newcode780
src/deoptimizer.cc:780: int frame_index = static_cast<int>(i);
Remove this static_cast and change the functions below to take size_t
index.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.cc#newcode2644
src/deoptimizer.cc:2644: int32_t i32_value =
static_cast<int32_t>(integral_value_);
Use a bit_cast<int32_t> here, or if you turn the fields into a union,
add an int32_t field to the union to access that value.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h
File src/deoptimizer.h (right):
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode114
src/deoptimizer.h:114: // The following is supposed to be a union.
Why not use a union here?
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode135
src/deoptimizer.h:135: Function,
Add k prefix to these constants.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode152
src/deoptimizer.h:152: class ValueIterator {
Can we have STL naming here? I.e. rename ValueIterator to iterator?
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode785
src/deoptimizer.h:785: intptr_t registers_[Register::kNumRegisters];
Nit: these should be private.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode1207
src/deoptimizer.h:1207:
Nit: redundant empty line.
https://codereview.chromium.org/1136223004/
--
--
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.
