https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.cc#newcode1979
src/hydrogen-instructions.cc:1979: * (base - constant_offset) &
constant_and_mask
Why do we not also want to match "base + constant_offset" and "base -
constant_offset"?

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.cc#newcode2003
src/hydrogen-instructions.cc:2003: if
(bitwise->right()->IsInteger32Constant()) {
If you get the mask first, before matching AND or OR, then you don't
need mask_location, and probably also don't need allow_offset.

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.cc#newcode2239
src/hydrogen-instructions.cc:2239: HBasicBlock* current_branch,
If I understand this code correctly now, this is the "true" target of a
hypothetical branch comparing the induction variable with "token"
against some limit. If that's true, I suggest "true_target" and
"false_target" for the basic block parameter names.

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.h#newcode3201
src/hydrogen-instructions.h:3201: void
UpdateAdditionalLimit(InductionVariableLimitUpdate* update) {
Also put this into the .cc file please.

And why are we swapping and not copying?

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.h#newcode3230
src/hydrogen-instructions.h:3230: bool
lower_limit_is_non_negative_constant() {
hacker_style is supposedly only for accessors.

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.h#newcode3244
src/hydrogen-instructions.h:3244: int32_t ComputeUpperLimit(int32_t
and_mask, int32_t or_mask) {
Put this one in the .cc file.

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/17568015/diff/31001/src/hydrogen.cc#newcode4453
src/hydrogen.cc:4453: if (FLAG_abcd_ivars) {
Indentation

https://codereview.chromium.org/17568015/diff/31001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

https://codereview.chromium.org/17568015/diff/31001/src/ia32/lithium-codegen-ia32.cc#newcode4382
src/ia32/lithium-codegen-ia32.cc:4382: void
LCodeGen::ApplyCheckIf(Condition cc, LBoundsCheck* check) {
I guess you were just using this for debugging on ia32, since the other
platforms don't have something similar, so perhaps you can take this out
now.

https://codereview.chromium.org/17568015/

--
--
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.


Reply via email to