[PATCH] D84220: [IPSCCP] Fix a bug that the "returned" attribute is not cleared when function is optimized to return undef

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:2056
+      if (!CB)
+        continue;
+      for (auto &arg : CB->args()) {
----------------
congzhe wrote:
> jdoerfert wrote:
> > congzhe wrote:
> > > jdoerfert wrote:
> > > > Funnily, if this branch would hit we have a problem. I mean, there is a use that could have an annotation like `returned` but we did make it UB now. Not that I think this branch should ever be taken anyway. I'd propose an assertion, @fhahn @efriedma WDYT ?
> > > Thank you, I guess I could replace the branch with something along the following line?
> > > ` assert(CB && "Use of zapped functions should be valid direct calls!");`
> > I think that would be better, yes.
> Thanks, updated accordingly.
Adding this assertion fails one test case: `llvm/test/Transforms/SCCP/indirectbr.ll`

This is because functions in `FuncZappedReturn` are functions that are zapped, but they may not necessarily have `returned` attribute for its arguments. Therefore `CB` can actually be `null` and hence fails assertion.

I can either revert back to the previous version (without assert), or I can add some code to make sure `FuncZappedReturn` contains functions that are zapped AND have `returned` attribute for its arguments.

PS. the specific test case that fails is the following:

```
; CHECK-LABEL: define internal i32 @indbrtest5(
; CHECK: ret i32 undef
define internal i32 @indbrtest5(i1 %c) {
entry:
  br i1 %c, label %bb1, label %bb2

bb1:
  br label %branch.block


bb2:
  br label %branch.block

branch.block:
  %addr = phi i8* [blockaddress(@indbrtest5, %target1), %bb1], [blockaddress(@indbrtest5, %target2), %bb2]
  indirectbr i8* %addr, [label %target1, label %target2]

target1:
  br label %target2

target2:
  ret i32 10
}


define i32 @indbrtest5_callee(i1 %c) {
; CHECK-LABEL: define i32 @indbrtest5_callee(
; CHECK-NEXT:    %r = call i32 @indbrtest5(i1 %c)
; CHECK-NEXT:    ret i32 10
  %r = call i32 @indbrtest5(i1 %c)
  ret i32 %r
}
```


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

https://reviews.llvm.org/D84220