[PATCH] D145209: [DAE] Don't change the return type if we have virtual musttail calls

[Nikita Popov via Phabricator via llvm-commits]

nikic added a reviewer: nikic.
nikic added inline comments.
Herald added a subscriber: StephenFan.


================
Comment at: llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp:526
+      // In addition, if the called function is virtual, we can't change
+      // the return type.
+      if (!TC->getCalledFunction())
----------------
mtrofin wrote:
> efriedma wrote:
> > Please explain why this matters.  Intuitively, whether a function is virtual shouldn't affect liveness... do we handle the non-virtual case somewhere else?
> Not sure I understand. It's about virtual musttail calls, not just virtual. "Liveness" in this pass means "it can't change".
> 
> Without the patch, the test fails in the validator, because the module is transformed to:
> 
> ```
> define internal void @test(ptr %fptr, i32 %a, i32 %b) {
>   %r = musttail call i32 %fptr(ptr %fptr, i32 %a, i32 0)
>   ret void
> }
> 
> ```
> 
> The error is what the test checks isn't present: `cannot guarantee tail call due to mismatched parameter counts`
> 
> The patch is actually incomplete - about to update it. This marking needs to be also propagated to users of the function.
> 
I believe Eli's point is that this is also true for non-virtual functions. For example, this example without virtual calls fails in the same way:
```
define internal i32 @test() {
  %r = musttail call i32 @foo()
  ret i32 %r
}

declare i32 @foo()
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145209