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

[Alexey Lapshin via Phabricator via llvm-commits]

avl marked an inline comment as done.
avl added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:838
+        if (isValidTRECandidate(CI))
+          HasValidCandidates = true;
+      }
----------------
laytonio wrote:
> Is there any reason to find and validate candidates now only to have to redo it when we actually perform the eliminations? If so, is there any reason this needs to follow a different code path than findTRECandidate? findTRECandidate is doing the same checks, except for canMoveAboveCall and canTransformAccumulatorRecursion, which should probably be refactored into findTRECandidate from eliminateCall anyway. If not then all of this code goes away and we're left with the same canTRE as in trunk.
We are enumerating all instructions here, so we could understand if there are not TRE candidates and stop earlier. That is the reason for doing it here.

I agree that findTRECandidate should be refactored to have the same checks as here. 

What do you think is better to do:

- leave early check for TRE candidates in canTRE or remove it
- refactor findTRECandidate or leave it as is

?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82085