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

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis marked 2 inline comments as done.
baziotis added inline comments.


================
Comment at: llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll:12
-; CHECK-NEXT:    [[A_0:%.*]] = load i32, i32* null
-; CHECK-NEXT:    br i1 false, label [[T:%.*]], label [[F:%.*]]
 ; CHECK:       T:
----------------
jdoerfert wrote:
> We should also repair this test. pass a valid pointer into `callee`, e.g., an argument. 
Ok,yes. Btw, the `nonnull` and `dereferenceable` attributes for `%A` are wrong right? I guess these are the kind of checks for violations you have proposed that AAUB could do.


================
Comment at: llvm/test/Transforms/Attributor/IPConstantProp/PR26044.ll:9
+;    live value is from %entry. Right now, AAIsDead does not
+;    interact with AAUB to know about the dead for.cond1.
 define void @fn2(i32* %P) {
----------------
jdoerfert wrote:
> baziotis wrote:
> > jdoerfert wrote:
> > > This test is actually really broken. The function can never be executed w/o causing UB. We need to fix these tests to have some behavior. 
> > Did I break anything? As far as I could understand, this was true before as well.
> > Or just a general comment? I personally think that now we have `AAUB`, it's ok to have "always UB" functions on tests that are supposed to test UB (like in the `undefined_behavior.ll`) but here it may be irrelevant to always have UB.
> The test was broken, the new AAUB just exposed it (more and more). Since this is supposed to test AAValueSimplify, we should remove the UB. Can you replace the undef in the branch with some argument value (which you need to add), and the undef in the phi with %p. Then let the jump go to some exit block that returns. At least that way we have defined behavior until the Attributor gets smart enough to realize the endless loop has no atomic side effect and can be removed as UB as well ;)
If I understood correctly, the code will look like this:
```
entry:
  br label %if.end

for.cond1:
  br i1 %C, label %if.end, label %if.end

if.end:
  %e.2 = phi i32* [ %P, %entry ], [ null, %for.cond1 ], [ null, %for.cond1 ]
  %0 = load i32, i32* %e.2, align 4
  ...
  br label %exit
exit:
  ret void
}
```
where `%C` is an argument. If we do that, there's no endless loop and the `%for.cond1` branch is unreachable (not because AAUB, but because there's no way to reach it).


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

https://reviews.llvm.org/D71960