[PATCH] D21010: Replace the implementation of ConstantRange::binaryAnd.

Nick Lewycky via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 5 23:38:51 PDT 2016


Sanjoy Das wrote:
> sanjoy added a comment.
>
> Hi Nick,
>
> I haven't read the code yet (will do that soon), but I wonder if you've tried the following approach: map a `ConstantRange` to `KnownOne` and `KnownZero`, use these to compute the binary and, and map the resultant `KnownOne` and `KnownZero` back to a `ConstantRange`.  That will probably not be easier in terms of code, but will make the code organization more obvious, and most of the interesting logic can be re-used for `binaryOr`.

That is the very first thing I tried. :-D

I still have the code (and tests!) for working fromKnownBits and 
toKnownBits methods if you want them. I was surprised at how small and 
efficient they were.

Extracting one test which would fail with that implementation:

   EXPECT_EQ(ConstantRange(APInt(8, 21), APInt(8, 25))
                 .binaryAnd(ConstantRange(APInt(8, 22), APInt(8, 26))),
             ConstantRange(APInt(8, 16), APInt(8, 25)));

NOTE: Closed ranges below, not half-open!

This is testing i8 [21, 24] & [22, 25]

21 0x15 0b10101 LHS only
22 0x16 0b10110 LHS and RHS
23 0x17 0b10111 LHS and RHS
24 0x18 0b11000 LHS and RHS
25 0x19 0b11001 RHS only

LHS range has known bits 0b1xxxx and RHS has known bits 0b1xxxx. The 
binary-and of those is 0b1xxxx, which produces the range i8 [16, 31], 
but there is no pair of values which, when and'd, produce 26, 27, 28, 
29, 30 or 31. You can get 16 by taking 21 from LHS and 24 from RHS, and 
24 by taking 24 from both ranges. i8 [21, 24] is the correct answer.

That example worked out too cleanly; the lower was equal to 'choosing 0 
for unknown bits' and the upper-1 was the umin of the unsigned-max's of 
each range. To dispel a few patterns up front, i8 [4, 5] & [8, 9] is an 
example where the new upper-1 is 1 not 5, while i8 [5, 6] & [5, 6] has a 
new lower of 4, but i8 [5, 6] & [5] has a new lower of 5.

And since I have it on my brain, here's a little more about wrapped 
ranges. I made an error thinking that I could 'unroll' wrapped ranges, 
turning i8 [255, 0] into i9 [255, 256], binary-and those, and then 
truncate the resulting range. Not so! It's correct, but it does not 
produce the smallest range. Correctly:
     i8 [255, 0] & i8 [255, 0]
   = i8 {255&255, 255&0, 0&255, 0&0}  (curly braces for sets)
   = i8 {255, 0, 0, 0}
   = i8 {255, 0}
   = i8 [255, 0].
With the unrolling:
     i8 [255, 0] & i8 [255, 0]
--> i9 [255, 256] & i9 [255, 256]
   = i9 {255&255, 255&256, 256&255, 256&256}
   = i9 {255, 0, 0, 256}  (here be dragons!)
   = i9 [0, 256]  (this range includes 1 and 2 and 3, ...)
--> i8 full-set

My current approach gets this particular case right, and I'm pretty sure 
it always produces ranges that cover all resulting values, but I'm not 
sure it always produces the smallest range.

Nick


More information about the llvm-commits mailing list