[llvm-commits] [llvm] r75357 - in /llvm/trunk: include/llvm/Support/ConstantRange.h lib/Support/ConstantRange.cpp lib/Transforms/Scalar/PredicateSimplifier.cpp
Duncan Sands
baldrick at free.fr
Sat Jul 11 20:28:46 PDT 2009
Hi Nick,
> The property I'm looking for is that 'if your value is not within the
> range returned then it can't possibly satisfy the comparison'. I should
> write that then!
so if This = makeICmpRegion(Other) then for every T not in the range
This and for every O in the range Other, T Pred O is not true ?
I find this confusing - two negations in one definition is too much
for me! Suppose you defined makeICmpRegion2 by:
if This = makeICmpRegion2(Other) then for every T in This and for
every O in Other, T pred O is true. "This" is chosen maximal.
That seems simpler to me (but maybe not to you :) ) Then these methods
satisfy:
makeICmpRegion(Other, Pred) =
complement makeICmpRegion2(Other, opposite Pred),
so there would be no problem doing everything in terms of
makeICmpRegion2 instead. I think the routine defined for the world in
ConstantRange.h should be "makeICmpRegion2" (probably renamed to
makeICmpRegion) and not the current version, because it is conceptually
simpler.
> After some tweaking, the new comment reads:
>
> /// makeICmpRegion - Produce the smallest range that contains all values
> that
> /// might satisfy the comparison specified by Pred when compared to any
> value
> /// contained within Other.
This is indeed an improvement, but since the exact semantics are
important then I think you should be even more explicit, using
quantifiers as above, perhaps at the end of the comment. By the way,
does a unique smallest range always exist (the comment implicitly
suggests that one does)?
>>> + case ICmpInst::ICMP_NE:
>>> + if (CR.isSingleElement())
>>> + return ConstantRange(CR.getUpper(), CR.getLower());
>>> + return ConstantRange(W);
>> Shouldn't this just return the complementary range? I don't
>> understand why you distinguish between the singleton case and
>> the general case.
I had misunderstood how this method is defined. This also explains
my other comments :) By the way, if CR is the empty range then you
should return the empty range here (since you return the smallest
possible range).
Ciao,
Duncan.
More information about the llvm-commits
mailing list