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

2020-07-13 Thread Alexey Lapshin via cfe-commits
Hi David,   Thank you for the information. I would revert that commit and work on the fix.   Thank you, Alexey. >Воскресенье, 12 июля 2020, 12:44 +03:00 от David Zarzycki via Phabricator >: >  >davezarzycki added a comment. > >Hello. I have an auto-bisecting multi-stage bot that is failing on

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

2020-07-12 Thread David Zarzycki via Phabricator via cfe-commits
davezarzycki added a comment. Hello. I have an auto-bisecting multi-stage bot that is failing on two after this change. Can we please revert this or commit a quick fix? FAIL: Clang :: CXX/class/class.compare/class.spaceship/p1.cpp (6232 of 64222) TEST 'Clang ::

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

2020-07-11 Thread Alexey Lapshin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf7907e9d223d: [TRE] allow TRE for non-capturing calls. (authored by avl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/

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

2020-07-09 Thread Alexey Lapshin via Phabricator via cfe-commits
avl added a comment. Thank you, for the review. 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

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

2020-07-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085

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

2020-07-09 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 276799. avl added a comment. addressed comments: added test for multiple recursive calls, removed duplicated check for operand bundles, simplified and commented tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-07-08 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked an inline comment as done. avl added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:474 + // Operand Bundles or not marked as TailCall. + if (CI->isNoTailCall() || CI->hasOperandBundles() || !CI->isTailCall()) return

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

2020-07-08 Thread 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.

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

2020-07-08 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 276591. avl added a comment. addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085 Files: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp

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

2020-07-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:94 /// If it contains any dynamic allocas, returns false. static bool canTRE(Function ) { // Because of PR962, we don't TRE dynamic allocas. avl wrote: >

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

2020-07-08 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked 3 inline comments as done. avl added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:94 /// If it contains any dynamic allocas, returns false. static bool canTRE(Function ) { // Because of PR962, we don't TRE dynamic allocas.

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

2020-07-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:94 /// If it contains any dynamic allocas, returns false. static bool canTRE(Function ) { // Because of PR962, we don't TRE dynamic allocas. If we're not

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

2020-07-02 Thread Alexey Lapshin via Phabricator via cfe-commits
avl added a comment. @efriedma What do you think about current state of this patch? Is it OK? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085 ___ cfe-commits

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

2020-07-02 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 275123. avl added a comment. rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085 Files: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp

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

2020-06-29 Thread Alexey Lapshin via Phabricator via cfe-commits
avl added a comment. ping. 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

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

2020-06-25 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 273457. avl added a comment. removed early check for TRE candidates from canTRE(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085 Files:

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

2020-06-25 Thread Alexey Lapshin via Phabricator via cfe-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: > avl wrote: > >

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

2020-06-25 Thread Layton Kifer via Phabricator via cfe-commits
laytonio added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:838 +if (isValidTRECandidate(CI)) + HasValidCandidates = true; + } avl wrote: > laytonio wrote: > > Is there any reason to find and validate

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

2020-06-25 Thread Alexey Lapshin via Phabricator via cfe-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

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

2020-06-25 Thread Layton Kifer via Phabricator via cfe-commits
laytonio added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:838 +if (isValidTRECandidate(CI)) + HasValidCandidates = true; + } Is there any reason to find and validate candidates now only to have to

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

2020-06-24 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 273174. avl added a comment. check valid TRE candidate into findTRECandidate()(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085 Files:

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

2020-06-24 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked an inline comment as done. avl added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:830 + !CI->isTailCall()) +return false; +} laytonio wrote: > Is this correct? I think we want to

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

2020-06-24 Thread Layton Kifer via Phabricator via cfe-commits
laytonio added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:816 + for (auto : F) { +for (Instruction : BB) { + if (AllocaInst *AI = dyn_cast()) { You can use `for (Instruction : instructions(F))` here.

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

2020-06-24 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 273029. avl added a comment. removed usages of AllocaDerivedValueTracker from canTRE(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085 Files:

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

2020-06-24 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked an inline comment as done. avl 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 :

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
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 : F.args()) { avl wrote:

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

2020-06-23 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked an inline comment as done. avl 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 :

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
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 : F.args()) { avl wrote:

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

2020-06-23 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 272829. avl edited the summary of this revision. avl added a comment. addressed comments: 1. removed PointerMayBeCaptured() used for CalledFunction. 2. rewrote CanTRE() to visiting instructions only once. 3. replaced areAllLastFuncCallsRecursive() with

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

2020-06-23 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked an inline comment as done. avl 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 :

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
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 : F.args()) { avl wrote:

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

2020-06-23 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked an inline comment as done. avl 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 :

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
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 : F.args()) { avl wrote:

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

2020-06-23 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked an inline comment as done. avl 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 :

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

2020-06-22 Thread Eli Friedman via Phabricator via cfe-commits
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 : F.args()) { avl wrote:

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

2020-06-22 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked 3 inline comments as done. avl added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:823 + +bool TailRecursionEliminator::canTRE(Function ) { + // The local stack holds all alloca instructions and all byval arguments.

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

2020-06-22 Thread Aditya Kumar via Phabricator via cfe-commits
hiraditya added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:801 +if (Branch->isUnconditional()) + if (ReturnInst *Ret = dyn_cast( + Branch->getSuccessor(0)->getFirstNonPHIOrDbg())) can we use isa<>

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

2020-06-22 Thread Layton Kifer via Phabricator via cfe-commits
laytonio added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:823 + +bool TailRecursionEliminator::canTRE(Function ) { + // The local stack holds all alloca instructions and all byval arguments. There is no need to pass the

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

2020-06-22 Thread Eli Friedman via Phabricator via cfe-commits
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 : F.args()) { avl wrote:

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

2020-06-22 Thread Alexey Lapshin via Phabricator via cfe-commits
avl marked 4 inline comments as done. avl added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:130 +IsNocapture = true; + else if (Function *CalledFunction = CB.getCalledFunction()) { +if

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

2020-06-22 Thread 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 &&

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

2020-06-22 Thread Alexey Lapshin via Phabricator via cfe-commits
avl updated this revision to Diff 272378. avl added a comment. 1. deleted code doing more strict tailcall marking. 2. left removal of "AllCallsAreTailCalls". 3. added check for non-capturing calls while tracking alloca. 4. re-titled the patch. Repository: rG LLVM Github Monorepo CHANGES