[PATCH] D145209: [DAE] Don't DAE if we musttail call a "live" (non-DAE-able) function

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

I *think* @rnk is out for a few weeks, @nikic, does the patch look reasonable to you?



================
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())
----------------
nikic wrote:
> 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()
> ```
Oh, I see. Let me update then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145209