This is an archive of the discontinued LLVM Phabricator instance.

[ValueTracking] Add range computation from dominating conditions
Needs ReviewPublic

Authored by aleksandr.popov on Jun 13 2023, 10:46 AM.

Details

Summary

Long time ago there was a patch which added support in ValueTracking for
inferring known bits of a value from dominating conditional expressions:
https://reviews.llvm.org/D7708. But later it was removed due to the lack
of profitable examples.

But now ValueTracking can compute overflow for binary expressions and
it can be useful to infer more precise range from dominating conditions.
An example is the recently landed LICM optimization:
https://reviews.llvm.org/D148001.

Tests in the Transforms/LICM/hoist-add-sub.ll show the LICM benefits of
ValueTracking improving.

Diff Detail

Event Timeline

aleksandr.popov requested review of this revision.Jun 13 2023, 10:46 AM
aleksandr.popov edited the summary of this revision. (Show Details)Jun 13 2023, 11:26 AM
goldstein.w.n added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
6039

logicalor aswell?

6061

All of the above can be:

CmpInst::Predicate Pred;
const APInt * C;
if(match(Condition, m_c_ICmp(Pred, m_Specific(V), m_APInt(C))) && C->getBitWidth() == BitWidth)
   return ConstantRange::makeExactICmpRegion(Pred, *C);
6061

m_c_ICmp is also probably not needed b.c we cannoniclize constants to the RHS.

6083

Think you need to check that Node != nullptr.

6089

nit: assert message.

6102

Should this iterator through successors?

Сhanges according to review comments and test update

aleksandr.popov marked 4 inline comments as done.Jun 14 2023, 8:08 AM
aleksandr.popov added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
6039

I've added a comment that here we compute value's range from the conditional expression which must be true to reach this value. Thus, we process LogicalAnd only

6061

Thanks, good point!

6102

This check can be expensive, thus we execute it only for potentially profitable conditions: when the condition must be true to reach the value.

aleksandr.popov marked an inline comment as done.

Added comments

goldstein.w.n added inline comments.Jun 14 2023, 11:45 AM
llvm/lib/Analysis/ValueTracking.cpp
6053

I don't think this check is needed. LHS/RHS will always be the same type.

Removed unnecessary check

aleksandr.popov marked an inline comment as done.Jun 15 2023, 5:44 AM
XChy added a subscriber: XChy.Jul 30 2023, 1:52 PM