[PATCH] D60031: Split tailcallelim into tailcallmark and tailcallelim

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

In D60031#1454360 <https://reviews.llvm.org/D60031#1454360>, @uenoku wrote:

> Yes, tailcallmark is not moved after memcpyopt in this patch. 
>  I was going to make another patch for this change.
>
> So this patch only split the pass and there is nothing to be changed in results.


Fair point.

Except for the things I noted in the comments, this LGTM.



================
Comment at: lib/Transforms/Scalar/TailRecursionElimination.cpp:528
+  // true if all call instruction which can be reachble entry block are marked
+  // as "tail"
 
----------------
Function behavior descriptions go usually above the function (3 x /). See `canMoveAboveCall` below for an example.


================
Comment at: lib/Transforms/Scalar/TailRecursionElimination.cpp:530
 
+  for (auto *BB : depth_first(&F)) {
+    for (auto &I : *BB) {
----------------
Please use `BasicBlock` and `Instruction` instead of `auto`, it is not helping to hide the type.


================
Comment at: lib/Transforms/Scalar/TailRecursionElimination.cpp:535
+        return false;
+      }
+    }
----------------
I'd remove the braces around a single return stmt.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60031