Note that all comments for ARM apply to other platforms, too.
Please note that I have a pretty high review load, so I can't always
guarantee a
24-hour review turn around. I think you should work under the assumption
that
reviews might take several days, although I will try to keep the turn around
time for reviews as low as possible.
https://codereview.chromium.org/21042003/diff/14001/src/arm/lithium-arm.h
File src/arm/lithium-arm.h (right):
https://codereview.chromium.org/21042003/diff/14001/src/arm/lithium-arm.h#newcode216
src/arm/lithium-arm.h:216: }
Build the constructor like this instead:
LInstruction() : environment_(NULL), hydrogen_value_(NULL)
: bit_field_(IsCallBits::encode(false)) {
set_position(RelocInfo::kNoPosition);
}
https://codereview.chromium.org/21042003/diff/14001/src/arm/lithium-arm.h#newcode273
src/arm/lithium-arm.h:273: bool ClobbersTemps() const { return
IsCallBits::decode(bit_field_); }
There should only be a single function that explicitly uses
"IsCallBits::decode(bit_field_)", and that should be a public function
"bool IsCall() const { return IsCallBits::decode(bit_field_); }" The
other usages in this class should call the wrapper function IsCall
rather than decoding the bits directly.
https://codereview.chromium.org/21042003/diff/14001/src/arm/lithium-arm.h#newcode303
src/arm/lithium-arm.h:303:
coding standard nit: Please put the class definitions of the BitFields
here, not below.
https://codereview.chromium.org/21042003/diff/14001/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):
https://codereview.chromium.org/21042003/diff/14001/src/arm/lithium-codegen-arm.cc#newcode299
src/arm/lithium-codegen-arm.cc:299: if (pos >=0) RecordPosition(pos);
Since code is ouput linearly, I think it makes sense to have a
old_position member on the codegen object and a method
RecordAndUpdatePosition(pos) that has the logic that encapsulates the
logic for eliding repeating positions, and call that thing here and
above. Same applies to other platforms.
https://codereview.chromium.org/21042003/
--
--
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.
