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 t
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 ::
CXX/clas
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/
https://reviews.llvm.org/
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
https://
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
___
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
https://reviews.llvm.org
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 nullptr
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.
+
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
llvm/tes
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 &F) {
// Because of PR962, we don't TRE dynamic allocas.
avl wrote:
> ef
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 &F) {
// Because of PR962, we don't TRE dynamic allocas.
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 &F) {
// Because of PR962, we don't TRE dynamic allocas.
If we're not go
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 ma
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
llvm/test/Transform
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
https://lists.llvm.org/cgi-bi
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:
llvm/lib/Transforms/Scalar/TailR
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:
> > laytonio
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
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
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 re
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:
llvm/lib/Transforms/Scalar/TailRe
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
laytonio added inline comments.
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:816
+ for (auto &BB : F) {
+for (Instruction &I : BB) {
+ if (AllocaInst *AI = dyn_cast(&I)) {
You can use `for (Instruction &I : instructions(F))` here.
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:
llvm/lib/Transforms/Scalar/
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 &Arg : F.ar
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 wrot
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 &Arg : F.ar
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 wrot
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 isInTREPos
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 &Arg : F.ar
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 wrot
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 &Arg : F.ar
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 wrot
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 &Arg : F.ar
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 wrot
avl marked 3 inline comments as done.
avl added inline comments.
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:823
+
+bool TailRecursionEliminator::canTRE(Function &F) {
+ // The local stack holds all alloca instructions and all byval arguments.
---
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<> he
laytonio added inline comments.
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:823
+
+bool TailRecursionEliminator::canTRE(Function &F) {
+ // The local stack holds all alloca instructions and all byval arguments.
There is no need to pass th
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 wrot
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 (CalledFunction->getB
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 &&
---
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 SINCE
42 matches
Mail list logo
