[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:130
+            IsNocapture = true;
+          else if (Function *CalledFunction = CB.getCalledFunction()) {
+            if (CalledFunction->getBasicBlockList().size() > 0 &&
----------------
Please don't add code to examine the callee; if we're not deducing nocapture appropriately, we should fix that elsewhere.


================
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:335
+        II->getIntrinsicID() == Intrinsic::assume)
+      return true;
+
----------------
What is the new handling for lifetime.end/assume doing?


================
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825
+  // The local stack holds all alloca instructions and all byval arguments.
+  AllocaDerivedValueTracker Tracker;
+  for (Argument &Arg : F.args()) {
----------------
Do you have to redo the AllocaDerivedValueTracker analysis?  Is it not enough that the call you're trying to TRE is marked "tail"?


================
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:852
+
+        // Do not do TRE if exists recursive calls which are not last calls.
+        if (!isTailBlock(CI->getParent()) ||
----------------
I thought we had some tests where we TRE in the presence of recursive calls, like a simple recursive fibonacci.  Am I misunderstanding this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82085