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
On 2013/07/11 16:39:10, titzer wrote:
Why do we not also want to match "base + constant_offset" and "base -
constant_offset"?
Because collapsing those patterns is the job of the other array bounds
elimination pass.
Of course we could also handle hoisting of "base + constant" patterns
but I did not think that the added complexity was worth it (at that
point we would end up with the "all encompassing" idef based approach).
https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.cc#newcode2003
src/hydrogen-instructions.cc:2003: if
(bitwise->right()->IsInteger32Constant()) {
On 2013/07/11 16:39:10, titzer wrote:
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.
True about mask, but we still want to allow those offsets in the
"bitwise and" case.
To be fair we could allow *anything* in the bitwise and case (the
constant operand is an upper limit anyway) but I did not want to go that
far.
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) {
On 2013/07/11 16:39:10, titzer wrote:
Also put this into the .cc file please.
And why are we swapping and not copying?
Done.
We are swapping because of the need for backtracking, see added comment.
https://codereview.chromium.org/17568015/diff/31001/src/hydrogen-instructions.h#newcode3230
src/hydrogen-instructions.h:3230: bool
lower_limit_is_non_negative_constant() {
On 2013/07/11 16:39:10, titzer wrote:
hacker_style is supposedly only for accessors.
Done.
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) {
On 2013/07/11 16:39:10, titzer wrote:
Put this one in the .cc file.
Done.
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) {
On 2013/07/11 16:39:10, titzer wrote:
Indentation
Done.
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) {
On 2013/07/11 16:39:10, titzer wrote:
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.
This needs to stay because it is a sort of "safety net": with this,
whenever we run tests with --debug_code we also fully check the bounds
check elimination.
As long as we test on ia32 IMHO it is not really necessary to port this
to other archs (but of course we could do it).
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.
