[PATCH] D86349: [Attributor] Fix IntegerRangeState::unionKnown

[Hideto Ueno via Phabricator via llvm-commits]

uenoku added inline comments.


================
Comment at: llvm/test/Transforms/Attributor/range.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes --check-attributes
-; RUN: opt -attributor -enable-new-pm=0 -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=25 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
-; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=31 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
----------------
okura wrote:
> jdoerfert wrote:
> > okura wrote:
> > > 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.
> > 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.
> 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?
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". 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86349/new/

https://reviews.llvm.org/D86349