Still LGTM.
I think it's a good idea to land the 1x1 decomposition to make progress
since it
already improves the range analysis for all binary bitwise ops.
As Stephen explained, it can be further extended, but I'd prefer this as a
separate change.
https://codereview.chromium.org/9156001/diff/2002/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/9156001/diff/2002/src/hydrogen-instructions.cc#newcode1317
src/hydrogen-instructions.cc:1317: result.ExtendRange(&lower, &upper);
On 2013/06/11 05:19:15, sra1 wrote:
On 2012/02/28 13:26:52, fschneider wrote:
> I'd simplify this by propagating the inputs kMaxInt and kMinInt into
ExtendRange
> and change it to return a Range object directy:
>
> return result.ExtendRange();
This looks a little strange but let me explain.
The code above is the 1x1 decomposition.
Each input range is 'decomposed' into 1 BitRange.
It is possible to do a more precise decompostion into several
BitRanges.
2 BitRanges might be the sweet-spot since it prevents change-of-sign
polluting
the result.
E.g. [-2,3] = {xxxxxxxx} as one BitRange, but is {1111111x, 000000xx}
as two.
If you have a 2x2 decomposition, then you have 4 intermediate results.
[-2,3] ^ [-1,5] = {xxxxxxxx} ^ {xxxxxxxx} = xxxxxxxx
but
[-2,3] ^ [-1,5] = {1111111x, 000000xx} ^ {11111111,00000xxx}
result11 = 1111111x ^ 11111111 = 0000000x
result12 = 1111111x ^ 00000xxx = 11111xxx
result21 = 000000xx ^ 11111111 = 111111xx
result22 = 000000xx ^ 00000xxx = 00000xxx
These can be accumulated into a range as follows:
result11.ExtendRange(&lower, &upper); // 0, 1
result12.ExtendRange(&lower, &upper); // -8, 1
result21.ExtendRange(&lower, &upper); // -8, 1
result22.ExtendRange(&lower, &upper); // -8, 7
= [-8,7]
Do you think I should implement the 2x2 decomposition? It is more
complex but
does ensure that small signed integers are known to be smi.
Thanks for the explanation. The code just looked strange to me for the
1x1 case. It makes sense for the 2x2 case to write it this way.
https://codereview.chromium.org/9156001/diff/22003/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/9156001/diff/22003/src/hydrogen-instructions.cc#newcode2387
src/hydrogen-instructions.cc:2387: */
Remove commented code.
https://codereview.chromium.org/9156001/diff/22003/test/cctest/test-bitrange.cc
File test/cctest/test-bitrange.cc (right):
https://codereview.chromium.org/9156001/diff/22003/test/cctest/test-bitrange.cc#newcode1
test/cctest/test-bitrange.cc:1: // Copyright 2010 the V8 project
authors. All rights reserved.
2013
https://codereview.chromium.org/9156001/diff/22003/test/cctest/test-bitrange.cc#newcode176
test/cctest/test-bitrange.cc:176: CheckOp(0, 99, 4, 4, &BitRange::Or,
4, 127);
Add a test for xor as well, e.g. 00xx ^ 0100 => 01xx
https://codereview.chromium.org/9156001/
--
--
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.
