[PATCH] D71852: [Attributor] Reach optimistic fixpoint in AAValueSimplify when the value is constant or undef

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D71852#1795513 <https://reviews.llvm.org/D71852#1795513>, @baziotis wrote:

> I'm not super familiar with `AAValueSimplify`, but here's a perspective: The conceptual idea is that `AAValueSimplify` returns optimistic fixpoint if it actually changed the value.
>  We may not want to break that. Instead, keep the "contract" to whoever is using it to: Use that assuming that if you got a known value, then it means it changed from the original.
>  The implication being: Check for yourself if it is something that won't change (i.e. it is constant). One one hand that puts more logistics to the user. On the other hand, the code may become more
>  understandable as the check for constant (or undef) happens with regular LLVM values that anyone who is not familiar with the Attributor can understand what is happening (i.e. from looking the code, the reader knows that if the value is constant, it won't even go through `AAValueSimplify`).
>  **Edit**: Well, AFAIK obviously the user of `AAValueSimplify` will be another part of the Attributor, so the reader is supposed to be familiar with it. But still, I think the fact that we'll go through `AAValueSimplify` only if we have some kind of complicated value is makes the code more understandable.


Sorry, I didn't see your comment at first.

`AAValueSimplify` computes the best simplified value, it is considered "assumed" as long as we cannot prove it is correct, once that happens it is "known". In case of a constant (or undef which should actually be a constant), there is not much to do as it is the most simplified version (in almost all cases), thus we know it is a correct value.

@uenoku Thinking about it again, we could also not set the "invalid" bit when we call `indicatePessimisticFixpoint` and instead just set `SimplifiedAssociatedValue` to the associated one and make it an optimistic fixpoint.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:4072
     // TODO: add other stuffs
-    if (isa<Constant>(V) || isa<UndefValue>(V))
-      indicatePessimisticFixpoint();
+    if (isa<Constant>(V) || isa<UndefValue>(V)) {
+      SimplifiedAssociatedValue = &V;
----------------
Nit: I think Undef is a constant making the second part of the conditional obsolete.


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

https://reviews.llvm.org/D71852