[PATCH] D80991: [WIP][Attributor] AAPotentialValues Attribute

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added a comment.

First of all, great patch. Reading the description and the referenced diff, I can't find an explanation about what this patch tries to do, it would be good if you added that. My assumption is:

You try to deduce what potential values can an IR position value get. Currently, it supports only ints (with that assumption I wrote the comments above). Correct?

(Even if you agree with that, please add some explanation about the method you're following)



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:3057
+  /// If this is true, the value of \p Set is meaningless.
+  bool isFull;
+
----------------
This is something that Johannes can give more insight and also a description for the patch might help but: I guess that what you mean here by "full" is the "top" (or "bottom", depending on how you see it) of a lattice in a data-flow analysis.

Point being, "top" pretty much means "I don't know, it could be anything" and it's a common knowledge for people.

"Full" OTOH, especially without further explanation about what it means, is not that clear IMHO. e.g. can I insert stuff up to a certain point ? When does something becomes full if the set can grow infinitely large ?


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:3185
+  /// Set representing assumed set of potential values.
+  StateTy Assumed;
+};
----------------
okura wrote:
> jdoerfert wrote:
> > I'm not sure if we really want/need to track the known set here. Maybe I just don't understand what we currently track in it. Feel free to explain :)
> > If it is "just" to match the design of other states, don't worry. Take the full set as the worst state and remove the known stuff.
> `Known.IsFull` is always true in the current implementation, But when the bit width of an value is 1 or 2,  we can initialize known set as actual full-set (`isFull` is false).
> By this, the fixpoint state can be reached faster and we may be able to reduce the number of iterations. That's why I left known in here. Is this a bad idea?
I agree with Johannes. OTOH, I'm sorry but I did not understand your comment. First of all, regarding the bit-width, do you mean that e.g. if I have a bit-width of 1, then I know I have only two possible values and so I know the full set ? But what is the "actual" full set ? Even in that case, if you don't know which of the values you have, you still then have pessimistic fix-point (i.e. both values, i.e. `isFull = true`). No?




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

https://reviews.llvm.org/D80991