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

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I think I'd like to see a testcase where there are multiple recursive calls, but only one is a tail call that can be eliminated.



================
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:474
+  // Operand Bundles or not marked as TailCall.
+  if (CI->isNoTailCall() || CI->hasOperandBundles() || !CI->isTailCall())
     return nullptr;
----------------
The hasOperandBundles() check looks completely new; is there some test for it?

The `isNoTailCall()` check is currently redundant; it isn't legal to write "tail notail".  I guess it makes sense to guard against that, though.


================
Comment at: llvm/test/Transforms/TailCallElim/basic.ll:23
+; CHECK: call i32 @test1
+	%X = call i32 @test1()		; <i32> [#uses=1]
 	ret i32 %X
----------------
I'm not sure this is testing what it was originally supposed to.  I guess that's okay, but please fix the comment at least.


================
Comment at: llvm/test/Transforms/TailCallElim/tre-noncapturing-alloca-calls.ll:20
+; Function Attrs: nofree noinline norecurse nounwind uwtable
+define dso_local void @_Z15globalIncrementPKi(i32* nocapture readonly %param) local_unnamed_addr #0 {
+entry:
----------------
For the purpose of this testcase, we don't need the definition of _Z15globalIncrementPKi.


================
Comment at: llvm/test/Transforms/TailCallElim/tre-noncapturing-alloca-calls.ll:37
+; CHECK: br label %tailrecurse
+; CHECK-NOT: call void @_Z4testi
+; CHECK: ret
----------------
I think I'd prefer to just generate this with update_test_checks.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82085