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);
On 2015/06/03 04:02:17, Benedikt Meurer wrote:
Remove this static_cast and change the functions below to take size_t
index.
Yeah, I tried that, but in those functions we compare the frame_index to
output_count_, so I would have to cast there. (And I would prefer not to
change output_count_'s type because it is exposed to the
platform-specific part of the deoptimizer.)
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_);
On 2015/06/03 04:02:17, Benedikt Meurer wrote:
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.
Turned to unions.
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.
On 2015/06/03 04:02:17, Benedikt Meurer wrote:
Why not use a union here?
Done.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode135
src/deoptimizer.h:135: Function,
On 2015/06/03 04:02:17, Benedikt Meurer wrote:
Add k prefix to these constants.
Done.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode152
src/deoptimizer.h:152: class ValueIterator {
On 2015/06/03 04:02:17, Benedikt Meurer wrote:
Can we have STL naming here? I.e. rename ValueIterator to iterator?
Done.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode785
src/deoptimizer.h:785: intptr_t registers_[Register::kNumRegisters];
On 2015/06/03 04:02:17, Benedikt Meurer wrote:
Nit: these should be private.
The offset of the field is exposed to the platform-specific assembly
code.
https://codereview.chromium.org/1136223004/diff/160001/src/deoptimizer.h#newcode1207
src/deoptimizer.h:1207:
On 2015/06/03 04:02:17, Benedikt Meurer wrote:
Nit: redundant empty line.
Done.
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.
