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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
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:
> 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.
Please add a comment explaining why direct calls and indirect calls are handled differently.  (Is it related to the usage of getCalledFunction() in surveyUse?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145209