This will be used in a future change to ScalarEvolution.
Details
Diff Detail
Event Timeline
The logic looks correct, and I really like the test!
include/llvm/IR/ConstantRange.h | ||
---|---|---|
89–93 | Example in comments don't match function declaration? Did you want to make it an 'int WrappingFlags'? (I see that you actually use it as a bool, but I really like the named flags API shown in the comment much more.) | |
lib/IR/ConstantRange.cpp | ||
138 | use BitWidth here | |
140 | C.isMinValue() is fully defined in the header. | |
151 | I think that's locally obvious enough that we don't need an assert. :) |
include/llvm/IR/ConstantRange.h | ||
---|---|---|
89–93 | I initially did have it as a int WrappingFlags (and I forgot to update the comments when I changed this to use a bool), but using AddOperator::... here seemed arbitrary (for instance, is it okay to have BinOp be Instruction::Mul and pass in AddOperator::NoSignedWrap? However, I share your preference for named parameters :), so I'll happily change this back to use AddOperator::NoUnsignedWrap if you're okay with it. I can also introduce an enum inside ConstantRange if that sounds better. |
include/llvm/IR/ConstantRange.h | ||
---|---|---|
89–93 | Changed to use OverflowingBinaryOperator::NoXXXWrap. |
lib/IR/ConstantRange.cpp | ||
---|---|---|
138–139 | But the API is called makeNoWrapRegion, so I'd say calling it with OBO::Exact or other flags unrelated to wrapping is a misuse. (I don't have a very strong opinion on this, and will change it if you insist. |
lib/IR/ConstantRange.cpp | ||
---|---|---|
138–139 | Oh, good point. I was imagining generalizing this function for all the optimization flags, but right now it's clearly labelled for wrapping. We can change that later when it's required. |
I was using intersectWith incorrect. I've fixed the problem now (see PreciseIntersect).
include/llvm/IR/ConstantRange.h | ||
---|---|---|
91 | There is no OBO::UnsignedWrap?? | |
lib/IR/ConstantRange.cpp | ||
139 | ExclusiveIntersect? MinimalIntersect? SubsetIntersect? I don't feel like precise conveys meaning here (what's not precise about intersectWith? It's not about precision, it's about whether we choose to include all points in the intersection or choose to include only pointer in the intersection). |
There is no OBO::UnsignedWrap??