[PATCH] D13677: [IR] Fix bug in `ConstantRange::intersectWith`

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 12 19:49:02 PDT 2015



Nick Lewycky wrote:
> On 12 October 2015 at 18:22, Sanjoy Das <sanjoy at playingwithpointers.com
> <mailto:sanjoy at playingwithpointers.com>> wrote:
>
>     sanjoy abandoned this revision.
>     sanjoy added a comment.
>
>     This change is unnecessary and buggy.  I was working under the
>     assumption that everything in `X.intersectWith(Y)` has to belong to
>     both `X` and `Y`; whereas `X.intersectWith(Y)` is //allowed// to
>     return a superset of the actual mathematical intersection of `X` and
>     `Y`.  This change is buggy today because it will sometimes return a
>     proper subset of the mathematical intersection of `X` and `Y`.
>
>
> I'm thoroughly concerned that neither of us noticed this at first. This
> sort of thing calls for an API review.

I have to admit, the API is a little confusing.  It seemed very 
"natural" to me that intersectWith() would "round down" towards the 
empty set.  However, if it did that, then patterns like the ones in 
ScalarEvolution::getRange won't work.

> I'm glad though, because I was certain intersectWith was really, really
> bug free.

Me too!! :)

-- Sanjoy


More information about the llvm-commits mailing list