Page MenuHomePhabricator

[ValueTracking] Support vscale in computeConstantRange()
ClosedPublic

Authored by nikic on Thu, Mar 16, 4:52 AM.

Details

Summary

Add support for vscale in computeConstantRange(), based on vscale_range attributes. This allows simplifying based on the precise range, rather than a KnownBits approximation (which will be off by a factor of two for the usual case of a power of two upper bound).

Diff Detail

Event Timeline

nikic created this revision.Thu, Mar 16, 4:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Mar 16, 4:52 AM
nikic requested review of this revision.Thu, Mar 16, 4:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Mar 16, 4:52 AM
sdesmalen added inline comments.Thu, Mar 16, 5:13 AM
llvm/lib/Analysis/ValueTracking.cpp
1160–1164

Is it possible to create test for this?

nikic updated this revision to Diff 505791.Thu, Mar 16, 6:30 AM

Rebase over additional tests for the poison case. In this case all comparisons just fold to true (but could also fold to any other value).

reames accepted this revision.Thu, Mar 16, 7:41 AM

LGTM

This revision is now accepted and ready to land.Thu, Mar 16, 7:41 AM

In this case all comparisons just fold to true (but could also fold to any other value).

What is the reason that the IR doesn't explicitly return poison in that case? (i.e. ret i1 poison)

nikic added a comment.Thu, Mar 16, 7:50 AM

In this case all comparisons just fold to true (but could also fold to any other value).

What is the reason that the IR doesn't explicitly return poison in that case? (i.e. ret i1 poison)

No particular reason, this just hasn't been implemented. (Would need a special case for empty ranges in simplifyICmpWithConstant.)

sdesmalen accepted this revision.Thu, Mar 16, 8:16 AM

In this case all comparisons just fold to true (but could also fold to any other value).

What is the reason that the IR doesn't explicitly return poison in that case? (i.e. ret i1 poison)

No particular reason, this just hasn't been implemented. (Would need a special case for empty ranges in simplifyICmpWithConstant.)

Fair enough, I was mostly wondering if there was a fundamental reason for it or if my understanding of poison was lacking here :) Thanks for clarifying!

This revision was landed with ongoing or failed builds.Fri, Mar 17, 2:05 AM
This revision was automatically updated to reflect the committed changes.