This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Fix IntegerRangeState::unionKnown
Needs ReviewPublic

Authored by okura on Aug 21 2020, 6:28 AM.

Details

Summary

Currently, the assumed range is taken union with a new known range in IntegerRangeState::unionKnown.
However, that is unnecessary because an assumed range is guaranteed to be subsumed by a known range.

Diff Detail

Event Timeline

okura created this revision.Aug 21 2020, 6:28 AM
Herald added a reviewer: homerdin. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
okura requested review of this revision.Aug 21 2020, 6:28 AM
okura added inline comments.Aug 21 2020, 6:30 AM
llvm/test/Transforms/Attributor/range.ll
2

An assumed range for the argument %depth of @rec in this test grows one by one during iterations. So we need a very big number of iterations to reach a fixpoint and I have difficulty in using attributor-max-iterations-verify option here.

jdoerfert added inline comments.Aug 21 2020, 9:36 AM
llvm/test/Transforms/Attributor/range.ll
2

You have to detect this and abort. We have similar situations in other places, e.g. dereferenceability and alignment, take a look and give it a try, we can discuss this further as well.

okura added inline comments.Aug 24 2020, 6:25 AM
llvm/test/Transforms/Attributor/range.ll
2

I understood that circular reasoning was caught by traversing values with stripAndAccumulateMinimalOffsets in AADereferenceable. To catch such circular reasoning in AAValueConstantRange, I think we need to track the dependence graph. (i.e. we need to check if there is a loop that contains the considered AA.)
Are my understanding and direction correct?

uenoku added inline comments.Aug 28 2020, 8:31 AM
llvm/test/Transforms/Attributor/range.ll
2

I might be wrong but I think the current reasoning around stripAndAccumulateMinimalOffsets looks unsound since we can say "Value V is in the range of [L, R]" by using ConstantRangeAA but not "Value V must become L in the execution".