[PATCH] D71960: [Attributor] AAUndefinedBehavior: AAValueSimplify in memory accessing instructions.

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added a comment.

In D71960#1797942 <https://reviews.llvm.org/D71960#1797942>, @uenoku wrote:

> > There's one test case that is worrying: IPConstantProp/PR26044.ll. With this patch, a from undef becomes load from null and I don't see the reason.
>
> I think this is no problem. It is because of on-demand creation. It is the first time to create `AAValueSimplify` to a pointer operand. The deduction is like this:
>
> 1. The assumption which claims "`for.cond1` is dead" is rejected because `AAIsDead` doesn't know there is UB.
> 2. `%e.2` is simplified to `null` because %e.2 can be `null` or `undef`.
> 3. `%e.2` is replaced with `null` in `manifest`
>
>   So once `AAIsDead` can get to interact with `AAUB`, the problem will be solved. Please add FIXME now.


Hmm, actually by looking again the code, I think that `null` //is// the correct value.
>From looking at the IR, without thinking what Attributor does, it seems that the only valid value is `null` (because it comes from entry while the others are
unreachable).
Now, the more interesting part is actually looking what the Attributor does. If I followed the code correctly, previously we didn't query `AAValueSimplify`.
What happens if we do (and correct me if I'm wrong, it seems useful to me to know how it works) is that `%e.2` is a floating value,
which calls `genericValueTraversal()`, which for the `phi` node, calls `VisitValueCB` with the 3 incoming values: `null`, `undef` and `undef`.
This in turn calls `checkAndUpdate()` which ignores `UndefValue` and finalizes on `null` (but if we had other different values, this:

  if (AccumulatedSimplifiedValue.hasValue())
    return AccumulatedSimplifiedValue == QueryingValueSimplified;

in `checkAndUpdate()` would return `false` because of conflicting values, effectively indicating pessimistic fixpoint).

Now, other than that, I guess that with this:

> So once `AAIsDead` can get to interact with `AAUB`, the problem will be solved.

you meant that when `AAIsDead` will interact with `AAUB`, the former will ignore the 2 `phi`s in `genericValueTraversal()` since it will consider them dead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71960