efriedma added inline comments.
================
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()) {
----------------
avl wrote:
> efriedma wrote:
> > Do you have to redo the AllocaDerivedValueTracker analysis? Is it not
> > enough that the call you're trying to TRE is marked "tail"?
> >Do you have to redo the AllocaDerivedValueTracker analysis?
>
> AllocaDerivedValueTracker analysis(done in markTails) could be reused here.
> But marking, done in markTails(), looks like separate tasks. i.e. it is
> better
> to make TRE not depending on markTails(). There is a review for this -
> https://reviews.llvm.org/D60031
> Thus such separation looks useful(To not reuse result of markTails but have
> it computed inplace).
>
> > Is it not enough that the call you're trying to TRE is marked "tail"?
>
> It is not enough that call which is subject to TRE is marked "Tail".
> It also should be checked that other calls does not capture pointer to local
> stack:
>
> ```
> // do not do TRE if any pointer to local stack has escaped.
> if (!Tracker.EscapePoints.empty())
> return false;
>
> ```
>
> It is not enough that call which is subject to TRE is marked "Tail". It also
> should be checked that other calls does not capture pointer to local stack:
If there's an escaped pointer to the local stack, we wouldn't infer "tail" in
the first place, would we?
================
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()) ||
----------------
avl wrote:
> efriedma wrote:
> > I thought we had some tests where we TRE in the presence of recursive
> > calls, like a simple recursive fibonacci. Am I misunderstanding this?
> right, there is a testcase for fibonacchi:
>
> llvm/test/Transforms/TailCallElim/accum_recursion.ll:@test3_fib
>
> areAllLastFuncCallsRecursive() checking works well for fibonacci testcase:
>
>
> ```
> return fib(x-1)+fib(x-2);
>
> ```
>
> Since, Last funcs call chain is : fib()->fib()->ret.
> That check should prevent from such cases:
>
>
> ```
> return fib(x-1)+another_call()+fib(x-2);
> ```
>
>
> That check should prevent from such cases: return
> fib(x-1)+another_call()+fib(x-2);
Why do we need to prevent this?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82085/new/
https://reviews.llvm.org/D82085
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
