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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp:89
+bool isCalleeAnalyzable(const CallBase &CB) {
+  return CB.getCalledFunction() && !CB.getCalledFunction()->isDeclaration();
+}
----------------
Is `isDeclaration()` the right criterion here? Shouldn't we be checking for `hasLocalLinkage()`?


================
Comment at: llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp:1114
+  }
+}
+
----------------
It's not really clear to me why we need a separate loop to do this, and this can't be included as part of the existing liveness propagation. E.g. surveyFunction() already looks at all the users, and even checks for musttail callers there. Currently, that's only used to mark all the arguments as live, but not the return values. Would it be sufficient to force live return values at that point?


================
Comment at: llvm/test/Transforms/DeadArgElim/musttail-indirect.ll:1
+; RUN: opt -passes=deadargelim,verify -S < %s 2>&1 | FileCheck %s
+define internal i32 @test_caller(ptr %fptr, i32 %a, i32 %b) {
----------------
`verify` is unnecessary, as is `2>&1`, as is the CHECK-NOT. Verification failure causes a crash and will fail the test regardless of output.

Please also update update_test_checks.py.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145209