[PATCH] D83283: [Attributor] AAPotentialValues Interface

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:191

----------------
okura wrote:
> baziotis wrote:
> > Nit: Worst.
> > Good comment btw. Note that there are another 2 reasons that we might go to worst state. I would change it as follows:
> > "If the number of ... to a full set (the worse state)" -> "The set might (only) be forced to worst state, that is, to contain
> > every possible value (which effectively means that practically we know nothing about this IR position) in 3 cases:
> > 1) We surpassed the \p MaxPotentialValues threshold.
> > 2) We tried to initialize on a Value that we can't handle (e.g. an operator we don't currently handle)
> > 3) During the iterations, this Value was affected (e.g. because of an operation) by a Value that is in
> > worst state.
> > 
> > I'm sure about the first 2, I have to think a little bit about whether the 3rd is good. What do you think for this
> > as a doc comment?
> Thank you for the great suggestion. I think this comment explains the AA succinctly and accurately. As for case 3, we can regard that many potential values are found and we surpass the threshold. So I think the first 2 cases are sufficient. 
Glad you liked it. I think that the third is important, because it lays out how different Values affect one another. For example,
if the current Value is in an operation with a Value that is in worst state, then the current also goes into worst state but not because of the threshold. 


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

https://reviews.llvm.org/D83283