The current implementation of Tail Recursion Elimination has a very restricted
pre-requisite: AllCallsAreTailCalls. i.e. it requires that no function
call receives a pointer to local stack. Generally, function calls that
receive a pointer to local stack but do not capture it - should not
break TRE. This fix allows us to do TRE if it is proved that no pointer
to the local stack is escaped. To make sure whether pointer escaped or not,
it examines called function.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
markTails function set IsTailcall bit for functions which are not
last calls:
It's OK to set "tail" on any call that satisfies these requirements (from https://llvm.org/docs/LangRef.html#call-instruction): "Both markers [tail and musttail] imply that the callee does not access allocas from the caller. The tail marker additionally implies that the callee does not access varargs from the caller."
"tail" does not mean that the call *must* be generated as a tail call. It just means that it's safe to generate it as a tail call if it turns out to be possible (e.g. if the compiler can prove that @noarg doesn't return, or if it can prove that all the code after the call to @noarg has no effect, or so on).
So I don't think there is a bug here that needs to be fixed.
It's OK to set "tail" on any call that satisfies these requirements (from https://llvm.org/docs/LangRef.html#call-instruction): "Both markers [tail and musttail] imply that the callee does not access allocas from the caller. The tail marker additionally implies that the callee does not access varargs from the caller."
"tail" does not mean that the call *must* be generated as a tail call. It just means that it's safe to generate it as a tail call if it turns out to be possible (e.g. if the compiler can prove that @noarg doesn't return, or if it can prove that all the code after the call to @noarg has no effect, or so on).
Yes, that is understood: ""tail" does not mean that the call *must* be generated as a tail call".
I intended to make picture consistent: when markTails sees function body, it is evident that the first call is not a tail call. I agree that a compiler "can prove that @noarg doesn't return, or if it can prove that all the code after the call to @noarg has no effect" and this first call could become tail-call. Probably the suitable strategy, in this case, would be to recalculate marking when it is broken(instead of creating false positive marks). There are other cases(compiler could add function calls), which could break existing marking and then would be necessary to recalculate marking.
Having many false positive "tail" marks could create a confusing picture.
I would describe the full problem which I am trying to solve.
(I assumed this patch would be the first one for that problem):
cat test.cpp
int count; __attribute__((noinline)) void globalIncrement(const int* param) { count += *param; } void test(int recurseCount) { if (recurseCount == 0) return; { int temp = 10; globalIncrement(&temp); } test(recurseCount - 1); }
TRE is not done for that test case. There are two reasons for that:
First is that AllocaDerivedValueTracker does not use the PointerMayBeCaptured interface, and it does not see that &temp is not escaped.
Second is that AllCallsAreTailCalls is used as a pre-requisite for making TRE.
So it requires that all calls would be marked as "tail". This looks too restricted.
Instead, it should check that "&temp" is not escaped in globalIncrement() and that "test"
is a tail recursive call not using allocas. I think the confusion happened exactly because "tail" marking was done for all calls(not for the real tailcalls).
Thus I planned to do the following fixes:
- cleanup "tail" marking.(this patch)
- do not use "AllCallsAreTailCalls" as a pre-requisite for TRE.(this patch).
- use PointerMayBeCaptured inside AllocaDerivedValueTracker.
What do you think about all of this?
It seems to me that it would be good to have consistent "tail" marking.
But if it does not look like a good idea, I could continue to point 2. and 3.
Relevant llvm-dev thread. Noncapture use of locals disabling TailRecursionElimination
All "tail" needs to mean is "this call does not reference the current function's stack." That's all, no more or less.
The relevant documentation and APIs are a bit confusing. A "tail" marking is a prerequisite for tailcall optimization, but it's not really related to any of the other prerequisites for tailcall optimization. Dropping the "tail" marking just because the call isn't in a tail position would involve a bunch of work adding and removing "tail" markings, for no benefit.
It makes sense to teach tail recursion elimination not to depend so heavily on tail markings. But I don't think that implies we want to mess with the markings themselves.
I think the confusion happened exactly because "tail" marking was done for all calls(not for the real tailcalls).
I don't think there's any confusion here. It's just using the tail markings as a conservative estimate of capturing because it has to compute them anyway. And TRE in general is simplistic code that nobody has spent much time looking at in a long time. There are very few TRE opportunities in C/C++ code anyway.
It makes sense to teach tail recursion elimination not to depend so heavily on tail markings. But I don't think that implies we want to mess with the markings themselves.
Ok. Thus I need to drop part of this patch related to more strict tail marking, and continue with part which changes pre-requisite for TRE.
- deleted code doing more strict tailcall marking.
- left removal of "AllCallsAreTailCalls".
- added check for non-capturing calls while tracking alloca.
- re-titled the patch.
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
145 | Please don't add code to examine the callee; if we're not deducing nocapture appropriately, we should fix that elsewhere. | |
332 | What is the new handling for lifetime.end/assume doing? | |
829 | Do you have to redo the AllocaDerivedValueTracker analysis? Is it not enough that the call you're trying to TRE is marked "tail"? | |
856 | I thought we had some tests where we TRE in the presence of recursive calls, like a simple recursive fibonacci. Am I misunderstanding this? |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
145 | Ok. | |
332 | They are just skipped. In following test case: call void @_Z5test5i(i32 %sub) call void @llvm.lifetime.end.p0i8(i64 24, i8* nonnull %1) #5 call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %0) #5 br label %return they are generated in between call and ret. It is safe to ignore them while checking whether transformation is possible. | |
829 |
AllocaDerivedValueTracker analysis(done in markTails) could be reused here.
It is not enough that call which is subject to TRE is marked "Tail". // do not do TRE if any pointer to local stack has escaped. if (!Tracker.EscapePoints.empty()) return false; | |
856 | 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. return fib(x-1)+another_call()+fib(x-2); |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 |
If there's an escaped pointer to the local stack, we wouldn't infer "tail" in the first place, would we? | |
856 |
Why do we need to prevent this? |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
827 | There is no need to pass the function here since its a member variable. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
827 | Ok. | |
829 | If function receives pointer to alloca then it would not be marked with "Tail". Then we do not have a possibility to understand whether this function receives pointer to alloca but does not capture it: void test(int recurseCount) { if (recurseCount == 0) return; int temp = 10; globalIncrement(&temp); test(recurseCount - 1); } test - marked with Tail. | |
856 | We do not. I misunderstood the canTransformAccumulatorRecursion(). That check could be removed. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 |
For the given code, TRE won't mark the recursive call "tail". That transform isn't legal: the recursive call could access the caller's version of "temp". |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 |
it looks like recursive call could NOT access the caller's version of "temp": test(recurseCount - 1); Caller`s version of temp is accessed by non-recursive call: globalIncrement(&temp); If globalIncrement does not capture the "&temp" then TRE looks to be legal for that case. globalIncrement() would not be marked with "Tail". test() would be marked with Tail. Thus the pre-requisite for TRE would be: tail-recursive call must not receive pointer to local stack(Tail) and non-recursive calls must not capture the pointer to local stack. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 | Can you give a complete IR example where we infer "tail", but TRE is illegal? Can you give a complete IR example, we we don't infer "tail", but we still do the TRE transform here? |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 |
there is no such example. Currently all cases where we infer "tail" would be valid for TRE.
For the following example current code base would not infer "tail" for _Z15globalIncrementPKi and as the result would not do TRE for _Z4testi. @count = dso_local local_unnamed_addr global i32 0, align 4 ; Function Attrs: nofree noinline norecurse nounwind uwtable define dso_local void @_Z15globalIncrementPKi(i32* nocapture readonly %param) local_unnamed_addr #0 { entry: %0 = load i32, i32* %param, align 4 %1 = load i32, i32* @count, align 4 %add = add nsw i32 %1, %0 store i32 %add, i32* @count, align 4 ret void } ; Function Attrs: nounwind uwtable define dso_local void @_Z4testi(i32 %recurseCount) local_unnamed_addr #1 { entry: %temp = alloca i32, align 4 %cmp = icmp eq i32 %recurseCount, 0 br i1 %cmp, label %return, label %if.end if.end: ; preds = %entry %0 = bitcast i32* %temp to i8* call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %0) #6 store i32 10, i32* %temp, align 4 call void @_Z15globalIncrementPKi(i32* nonnull %temp) %sub = add nsw i32 %recurseCount, -1 call void @_Z4testi(i32 %sub) call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %0) #6 br label %return return: ; preds = %entry, %if.end ret void } ; Function Attrs: argmemonly nounwind willreturn declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #2 ; Function Attrs: argmemonly nounwind willreturn declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #2 attributes #0 = { nofree noinline norecurse nounwind uwtable } attributes #1 = { nounwind uwtable } attributes #2 = { argmemonly nounwind willreturn } |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 | In your example, we don't infer "tail" for globalIncrement... but we do infer it for the recursive call to test(). I'm suggesting you could just check for "tail" on test(), instead of using AllocaDerivedValueTracker. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 | Checking only test() is not enough. There additionally should be checked globalIncrement(). It is correct that while checking test() there could be checked "Tail" flag. Which does not require using AllocaDerivedValueTracker. But while checking globalIncrement() there should be checked whether some alloca value escaped or not. "Tail" flag could not be used for that. AllocaDerivedValueTracker allow to do such check: Tracker.EscapePoints.empty() If we would not do check for globalIncrement then it is not valid to do TRE. Thus it seems we need to check globalIncrement for escaping pointer and we need to use AllocaDerivedValueTracker for that. |
addressed comments:
- removed PointerMayBeCaptured() used for CalledFunction.
- rewrote CanTRE() to visiting instructions only once.
- replaced areAllLastFuncCallsRecursive() with isInTREPosition().
I did not address request for not using AllocaDerivedValueTracker yet.
Since there is open question on it. I would address it as soon as the question would be resolved.
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 |
I'm not sure how to reconcile this. Are you saying we could infer "tail" in some case where TRE is illegal, but don't right now? Or are you saying that you plan to extend TRE to handle cases where we can't infer "tail" on the recursive call? |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 |
I think not exactly. more precise would probably be : "I am saying that plan to extend TRE to handle cases where we can't infer "tail" on the NON-recursive NON-last call" globalIncrement() is non-recursive non-last call in above example. But we need to check whether it captures argument pointer or not to decide whether it is OK to do TRE. To make things clear - I am suggesting instead of current pre-requisite for TRE : "All call sites are marked with Tail" to make following: "Recursive last calls are marked with "Tail", non-recursive non-last calls are proved to not capture alloca". For the above example it means : the requirement for test() should stay the same(should be marked with Tail). The requirement for globalIncrement() should be "does not capture alloca". |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 | "Recursive last calls are marked with 'tail'" implies "non-recursive non-last calls are proved to not capture alloca". |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
829 | I see, thank you for explanations. Transforms/TailCallElim/basic.ll:@test1 ; PR615. Make sure that we do not move the alloca so that it interferes with the tail call. define i32 @test1() { ; CHECK: i32 @test1() ; CHECK-NEXT: alloca %A = alloca i32 ; <i32*> [#uses=2] store i32 5, i32* %A call void @use(i32* %A) ; CHECK: tail call i32 @test1 %X = tail call i32 @test1() ; <i32> [#uses=1] ret i32 %X } I removed usages of AllocaDerivedValueTracker and corrected the test1 from Transforms/TailCallElim/basic.ll. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
844 | I tried to minimize changes and keep old logic here - but yes, it is better to move that check into findTRECandidate(). Will do. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
842 | 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. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
842 | 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:
? |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
842 | Yes we are iterating all the instructions here but, unless I am missing something, we would literally just be doing the checks twice for no reason. Look at it this way, best case scenario we have to check all possible candidates once, find none and we're done. Worst case, we check all possible candidates once, find one and have to check all possible candidates a second time. Where as if we remove the early checks we only ever have to check the candidates once. So we wouldn't really be stopping any earlier. As for refactoring findTRECandidate, I do think that should be done and we should strive to move all the failure conditions out of eliminateCall in order to avoid having to fold a return only to find out we didn't need to. But, I think that is out of the scope of this change, and if we do decide to keep the early checks here then we should say that findTRECandidate does a good enough job to consider this function as having valid candidates. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
842 |
yes. we would do check twice if there are TRE candidates. my idea was that number of cases when TRE is applicable less then when TRE is not applicable. Thus we would do double instruction navigation more often than double check for candidates. But, I did not measure real impact. Thus, let`s return old logic here as you suggested. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
95 | If we're not going to try to do TRE at all on calls not marked "tail", we can probably drop this check. | |
332 | It makes sense we can ignore lifetime.end on an alloca: we know the call doesn't refer to the alloca. (Maybe we should check that the pointer argument is pointing at an alloca? That should usually be true anyway, but better to be on the safe side, I guess.) I don't think it's safe to hoist assume without additional checks; I think we'd need to check that the call is marked "willreturn"? Since this is sort of tricky, I'd prefer to split this off into a followup. | |
810–811 | Can you move this FIXME into a more appropriate spot? |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
95 | It looks to me that original idea(PR962) was to avoid inefficient code which is generated for dynamic alloca. Currently there would still be generated inefficient code: Doing TRE for dynamic alloca requires correct stack adjustment to avoid extra stack usage. Please, consider the test case: #include <alloca.h> int count; __attribute__((noinline)) void globalIncrement(const int* param) { count += *param; } void test(int recurseCount) { if (recurseCount == 0) return; { int *temp = (int*)alloca(100); globalIncrement(temp); } test(recurseCount - 1); } Following is the x86 asm generated for the above test case in assumption that dynamic allocas are possible: .LBB1_2: movq %rsp, %rdi addq $-112, %rdi <<<<<<<<<<<<<< dynamic stack reservation, need to be restored before "jne .LBB1_2" movq %rdi, %rsp callq _Z15globalIncrementPKi addl $-1, %ebx jne .LBB1_2 So, it looks like we still have inefficient code here and it was a reason for avoiding TRE. | |
332 |
OK, I would add checking that the pointer argument of lifetime.end is pointing to an alloca.
Ok, I would split Intrinsic::assume into another review. | |
810–811 | OK. |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
95 | I guess we can leave this for a later patch. This isn't really any worse than the stack usage before TRE, assuming we can't emit a sibling call in the backend. And we could avoid this by making TRE insert stacksave/stackrestore intrinsics. But better to do one thing at a time. |
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.
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
474 | 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. | |
llvm/test/Transforms/TailCallElim/basic.ll | ||
24 | 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. | |
llvm/test/Transforms/TailCallElim/tre-noncapturing-alloca-calls.ll | ||
21 | For the purpose of this testcase, we don't need the definition of _Z15globalIncrementPKi. | |
38 | I think I'd prefer to just generate this with update_test_checks.py |
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | ||
---|---|---|
474 |
it is not new. it is copied from 245 line. Now, when patch changed from its original state all above conditions could be changed just to : if (!CI->isTailCall()) the test is Transforms/TailCallElim/deopt-bundle.ll
would add checking for that. |
addressed comments:
added test for multiple recursive calls,
removed duplicated check for operand bundles,
simplified and commented tests.
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/class/class.compare/class.spaceship/p1.cpp' FAILED ******************** Script: -- : 'RUN: at line 1'; /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem /tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify /home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp -fcxx-exceptions -- Exit Code: 134 Command Output (stderr): -- clang: /home/dave/s/lp/clang/lib/Basic/SourceManager.cpp:917: clang::FileID clang::SourceManager::getFileIDLoaded(unsigned int) const: Assertion `0 && "Invalid SLocOffset or bad function choice"' failed. PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump: 0. Program arguments: /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem /tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify /home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp -fcxx-exceptions 1. /home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:127:38: current parser token ',' 2. /home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:39:1: parsing namespace 'Deletedness' 3. /home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:123:12: parsing function body 'Deletedness::g' 4. /home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:123:12: in compound statement ('{}') #0 0x000000000359273f llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/tmp/_update_lc/t/bin/clang+0x359273f) #1 0x0000000003590912 llvm::sys::RunSignalHandlers() (/tmp/_update_lc/t/bin/clang+0x3590912) #2 0x0000000003592bb5 SignalHandler(int) (/tmp/_update_lc/t/bin/clang+0x3592bb5) #3 0x00007ffff7fa6a90 __restore_rt (/lib64/libpthread.so.0+0x14a90) #4 0x00007ffff7b3da25 raise (/lib64/libc.so.6+0x3ca25) #5 0x00007ffff7b26895 abort (/lib64/libc.so.6+0x25895) #6 0x00007ffff7b26769 _nl_load_domain.cold (/lib64/libc.so.6+0x25769) #7 0x00007ffff7b35e86 (/lib64/libc.so.6+0x34e86) #8 0x000000000375636c clang::SourceManager::getFileIDLoaded(unsigned int) const (/tmp/_update_lc/t/bin/clang+0x375636c) #9 0x0000000003ee0bbb clang::VerifyDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) (/tmp/_update_lc/t/bin/clang+0x3ee0bbb) #10 0x00000000037501ab clang::DiagnosticIDs::ProcessDiag(clang::DiagnosticsEngine&) const (/tmp/_update_lc/t/bin/clang+0x37501ab) #11 0x0000000003749fca clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) (/tmp/_update_lc/t/bin/clang+0x3749fca) #12 0x0000000004df0c60 clang::Sema::EmitCurrentDiagnostic(unsigned int) (/tmp/_update_lc/t/bin/clang+0x4df0c60) #13 0x0000000005092783 (anonymous namespace)::DefaultedComparisonAnalyzer::visitBinaryOperator(clang::OverloadedOperatorKind, llvm::ArrayRef<clang::Expr*>, (anonymous namespace)::DefaultedComparisonSubobject, clang::OverloadCandidateSet*) (/tmp/_update_lc/t/bin/clang+0x5092783) #14 0x0000000005091dba (anonymous namespace)::DefaultedComparisonAnalyzer::visitExpandedSubobject(clang::QualType, (anonymous namespace)::DefaultedComparisonSubobject) (/tmp/_update_lc/t/bin/clang+0x5091dba) #15 0x0000000005091b86 (anonymous namespace)::DefaultedComparisonVisitor<(anonymous namespace)::DefaultedComparisonAnalyzer, (anonymous namespace)::DefaultedComparisonInfo, (anonymous namespace)::DefaultedComparisonInfo, (anonymous namespace)::DefaultedComparisonSubobject>::visitSubobjects((anonymous namespace)::DefaultedComparisonInfo&, clang::CXXRecordDecl*, clang::Qualifiers) (/tmp/_update_lc/t/bin/clang+0x5091b86) #16 0x0000000005058c8c (anonymous namespace)::DefaultedComparisonAnalyzer::visit() (/tmp/_update_lc/t/bin/clang+0x5058c8c) #17 0x000000000505ab22 clang::Sema::DiagnoseDeletedDefaultedFunction(clang::FunctionDecl*) (/tmp/_update_lc/t/bin/clang+0x505ab22) #18 0x00000000053e60ed clang::Sema::CreateOverloadedBinOp(clang::SourceLocation, clang::BinaryOperatorKind, clang::UnresolvedSetImpl const&, clang::Expr*, clang::Expr*, bool, bool, clang::FunctionDecl*) (/tmp/_update_lc/t/bin/clang+0x53e60ed) #19 0x000000000514270a BuildOverloadedBinOp(clang::Sema&, clang::Scope*, clang::SourceLocation, clang::BinaryOperatorKind, clang::Expr*, clang::Expr*) (/tmp/_update_lc/t/bin/clang+0x514270a) #20 0x00000000050fbf49 clang::Sema::ActOnBinOp(clang::Scope*, clang::SourceLocation, clang::tok::TokenKind, clang::Expr*, clang::Expr*) (/tmp/_update_lc/t/bin/clang+0x50fbf49) #21 0x0000000004d52ccc clang::Parser::ParseRHSOfBinaryExpression(clang::ActionResult<clang::Expr*, true>, clang::prec::Level) (/tmp/_update_lc/t/bin/clang+0x4d52ccc) #22 0x0000000004d51be9 clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/tmp/_update_lc/t/bin/clang+0x4d51be9) #23 0x0000000004d60dba clang::Parser::ParseExpressionList(llvm::SmallVectorImpl<clang::Expr*>&, llvm::SmallVectorImpl<clang::SourceLocation>&, llvm::function_ref<void ()>) (/tmp/_update_lc/t/bin/clang+0x4d60dba) #24 0x0000000004d542d9 clang::Parser::ParsePostfixExpressionSuffix(clang::ActionResult<clang::Expr*, true>) (/tmp/_update_lc/t/bin/clang+0x4d542d9) #25 0x0000000004d55b95 clang::Parser::ParseCastExpression(clang::Parser::CastParseKind, bool, bool&, clang::Parser::TypeCastState, bool, bool*) (/tmp/_update_lc/t/bin/clang+0x4d55b95) #26 0x0000000004d51b89 clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/tmp/_update_lc/t/bin/clang+0x4d51b89) #27 0x0000000004d51ac9 clang::Parser::ParseExpression(clang::Parser::TypeCastState) (/tmp/_update_lc/t/bin/clang+0x4d51ac9) #28 0x0000000004d78368 clang::Parser::ParseExprStatement(clang::Parser::ParsedStmtContext) (/tmp/_update_lc/t/bin/clang+0x4d78368) #29 0x0000000004d76ba0 clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::ParsedStmtContext, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) (/tmp/_update_lc/t/bin/clang+0x4d76ba0) #30 0x0000000004d76614 clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::ParsedStmtContext, clang::SourceLocation*) (/tmp/_update_lc/t/bin/clang+0x4d76614) #31 0x0000000004d7ecd2 clang::Parser::ParseCompoundStatementBody(bool) (/tmp/_update_lc/t/bin/clang+0x4d7ecd2) #32 0x0000000004d7fcd0 clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) (/tmp/_update_lc/t/bin/clang+0x4d7fcd0) #33 0x0000000004cfacc0 clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) (/tmp/_update_lc/t/bin/clang+0x4cfacc0) #34 0x0000000004d28f2d clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, clang::DeclaratorContext, clang::SourceLocation*, clang::Parser::ForRangeInit*) (/tmp/_update_lc/t/bin/clang+0x4d28f2d) #35 0x0000000004cf9f32 clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) (/tmp/_update_lc/t/bin/clang+0x4cf9f32) #36 0x0000000004cf9938 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) (/tmp/_update_lc/t/bin/clang+0x4cf9938) #37 0x0000000004cf86fc clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (/tmp/_update_lc/t/bin/clang+0x4cf86fc) #38 0x0000000004d02c15 clang::Parser::ParseInnerNamespace(llvm::SmallVector<clang::Parser::InnerNamespaceInfo, 4u> const&, unsigned int, clang::SourceLocation&, clang::ParsedAttributes&, clang::BalancedDelimiterTracker&) (/tmp/_update_lc/t/bin/clang+0x4d02c15) #39 0x0000000004d0251a clang::Parser::ParseNamespace(clang::DeclaratorContext, clang::SourceLocation&, clang::SourceLocation) (/tmp/_update_lc/t/bin/clang+0x4d0251a) #40 0x0000000004d22f0a clang::Parser::ParseDeclaration(clang::DeclaratorContext, clang::SourceLocation&, clang::Parser::ParsedAttributesWithRange&, clang::SourceLocation*) (/tmp/_update_lc/t/bin/clang+0x4d22f0a) #41 0x0000000004cf7e39 clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (/tmp/_update_lc/t/bin/clang+0x4cf7e39) #42 0x0000000004cf6858 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, bool) (/tmp/_update_lc/t/bin/clang+0x4cf6858) #43 0x0000000004cf16ed clang::ParseAST(clang::Sema&, bool, bool) (/tmp/_update_lc/t/bin/clang+0x4cf16ed) #44 0x0000000003e3eb21 clang::FrontendAction::Execute() (/tmp/_update_lc/t/bin/clang+0x3e3eb21) #45 0x0000000003dba0e3 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/tmp/_update_lc/t/bin/clang+0x3dba0e3) #46 0x0000000003ee796b clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/tmp/_update_lc/t/bin/clang+0x3ee796b) #47 0x0000000002244636 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/tmp/_update_lc/t/bin/clang+0x2244636) #48 0x000000000224297d ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) (/tmp/_update_lc/t/bin/clang+0x224297d) #49 0x0000000002242619 main (/tmp/_update_lc/t/bin/clang+0x2242619) #50 0x00007ffff7b28042 __libc_start_main (/lib64/libc.so.6+0x27042) #51 0x000000000223f8ce _start (/tmp/_update_lc/t/bin/clang+0x223f8ce) /tmp/_update_lc/t/tools/clang/test/CXX/class/class.compare/class.spaceship/Output/p1.cpp.script: line 1: 4146089 Aborted /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem /tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify /home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp -fcxx-exceptions -- ******************** Testing: 0.. FAIL: Clang :: CXX/class/class.compare/class.eq/p2.cpp (6242 of 64222) ******************** TEST 'Clang :: CXX/class/class.compare/class.eq/p2.cpp' FAILED ******************** Script: -- : 'RUN: at line 1'; /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem /tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify /home/dave/s/lp/clang/test/CXX/class/class.compare/class.eq/p2.cpp -- Exit Code: 134 Command Output (stderr): -- clang: /home/dave/s/lp/clang/lib/Basic/SourceManager.cpp:917: clang::FileID clang::SourceManager::getFileIDLoaded(unsigned int) const: Assertion `0 && "Invalid SLocOffset or bad function choice"' failed. PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump: 0. Program arguments: /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem /tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify /home/dave/s/lp/clang/test/CXX/class/class.compare/class.eq/p2.cpp 1. /home/dave/s/lp/clang/test/CXX/class/class.compare/class.eq/p2.cpp:47:30: current parser token ')' 2. /home/dave/s/lp/clang/test/CXX/class/class.compare/class.eq/p2.cpp:30:13: parsing function body 'test' 3. /home/dave/s/lp/clang/test/CXX/class/class.compare/class.eq/p2.cpp:30:13: in compound statement ('{}') #0 0x000000000359273f llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/tmp/_update_lc/t/bin/clang+0x359273f) #1 0x0000000003590912 llvm::sys::RunSignalHandlers() (/tmp/_update_lc/t/bin/clang+0x3590912) #2 0x0000000003592bb5 SignalHandler(int) (/tmp/_update_lc/t/bin/clang+0x3592bb5) #3 0x00007ffff7fa6a90 __restore_rt (/lib64/libpthread.so.0+0x14a90) #4 0x00007ffff7b3da25 raise (/lib64/libc.so.6+0x3ca25) #5 0x00007ffff7b26895 abort (/lib64/libc.so.6+0x25895) #6 0x00007ffff7b26769 _nl_load_domain.cold (/lib64/libc.so.6+0x25769) #7 0x00007ffff7b35e86 (/lib64/libc.so.6+0x34e86) #8 0x000000000375636c clang::SourceManager::getFileIDLoaded(unsigned int) const (/tmp/_update_lc/t/bin/clang+0x375636c) #9 0x0000000003ee0bbb clang::VerifyDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) (/tmp/_update_lc/t/bin/clang+0x3ee0bbb) #10 0x00000000037501ab clang::DiagnosticIDs::ProcessDiag(clang::DiagnosticsEngine&) const (/tmp/_update_lc/t/bin/clang+0x37501ab) #11 0x0000000003749fca clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) (/tmp/_update_lc/t/bin/clang+0x3749fca) #12 0x0000000004df0c60 clang::Sema::EmitCurrentDiagnostic(unsigned int) (/tmp/_update_lc/t/bin/clang+0x4df0c60) #13 0x00000000050928b7 (anonymous namespace)::DefaultedComparisonAnalyzer::visitBinaryOperator(clang::OverloadedOperatorKind, llvm::ArrayRef<clang::Expr*>, (anonymous namespace)::DefaultedComparisonSubobject, clang::OverloadCandidateSet*) (/tmp/_update_lc/t/bin/clang+0x50928b7) #14 0x0000000005091dba (anonymous namespace)::DefaultedComparisonAnalyzer::visitExpandedSubobject(clang::QualType, (anonymous namespace)::DefaultedComparisonSubobject) (/tmp/_update_lc/t/bin/clang+0x5091dba) #15 0x0000000005091b86 (anonymous namespace)::DefaultedComparisonVisitor<(anonymous namespace)::DefaultedComparisonAnalyzer, (anonymous namespace)::DefaultedComparisonInfo, (anonymous namespace)::DefaultedComparisonInfo, (anonymous namespace)::DefaultedComparisonSubobject>::visitSubobjects((anonymous namespace)::DefaultedComparisonInfo&, clang::CXXRecordDecl*, clang::Qualifiers) (/tmp/_update_lc/t/bin/clang+0x5091b86) #16 0x0000000005058c8c (anonymous namespace)::DefaultedComparisonAnalyzer::visit() (/tmp/_update_lc/t/bin/clang+0x5058c8c) #17 0x000000000505ab22 clang::Sema::DiagnoseDeletedDefaultedFunction(clang::FunctionDecl*) (/tmp/_update_lc/t/bin/clang+0x505ab22) #18 0x00000000053e60ed clang::Sema::CreateOverloadedBinOp(clang::SourceLocation, clang::BinaryOperatorKind, clang::UnresolvedSetImpl const&, clang::Expr*, clang::Expr*, bool, bool, clang::FunctionDecl*) (/tmp/_update_lc/t/bin/clang+0x53e60ed) #19 0x000000000514270a BuildOverloadedBinOp(clang::Sema&, clang::Scope*, clang::SourceLocation, clang::BinaryOperatorKind, clang::Expr*, clang::Expr*) (/tmp/_update_lc/t/bin/clang+0x514270a) #20 0x00000000050fbf49 clang::Sema::ActOnBinOp(clang::Scope*, clang::SourceLocation, clang::tok::TokenKind, clang::Expr*, clang::Expr*) (/tmp/_update_lc/t/bin/clang+0x50fbf49) #21 0x0000000004d52ccc clang::Parser::ParseRHSOfBinaryExpression(clang::ActionResult<clang::Expr*, true>, clang::prec::Level) (/tmp/_update_lc/t/bin/clang+0x4d52ccc) #22 0x0000000004d51be9 clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/tmp/_update_lc/t/bin/clang+0x4d51be9) #23 0x0000000004d60dba clang::Parser::ParseExpressionList(llvm::SmallVectorImpl<clang::Expr*>&, llvm::SmallVectorImpl<clang::SourceLocation>&, llvm::function_ref<void ()>) (/tmp/_update_lc/t/bin/clang+0x4d60dba) #24 0x0000000004d4b29c clang::Parser::ParseCXXTypeConstructExpression(clang::DeclSpec const&) (/tmp/_update_lc/t/bin/clang+0x4d4b29c) #25 0x0000000004d57617 clang::Parser::ParseCastExpression(clang::Parser::CastParseKind, bool, bool&, clang::Parser::TypeCastState, bool, bool*) (/tmp/_update_lc/t/bin/clang+0x4d57617) #26 0x0000000004d51b89 clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/tmp/_update_lc/t/bin/clang+0x4d51b89) #27 0x0000000004d51ac9 clang::Parser::ParseExpression(clang::Parser::TypeCastState) (/tmp/_update_lc/t/bin/clang+0x4d51ac9) #28 0x0000000004d78368 clang::Parser::ParseExprStatement(clang::Parser::ParsedStmtContext) (/tmp/_update_lc/t/bin/clang+0x4d78368) #29 0x0000000004d76ba0 clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::ParsedStmtContext, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) (/tmp/_update_lc/t/bin/clang+0x4d76ba0) #30 0x0000000004d76614 clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::ParsedStmtContext, clang::SourceLocation*) (/tmp/_update_lc/t/bin/clang+0x4d76614) #31 0x0000000004d7ecd2 clang::Parser::ParseCompoundStatementBody(bool) (/tmp/_update_lc/t/bin/clang+0x4d7ecd2) #32 0x0000000004d7fcd0 clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) (/tmp/_update_lc/t/bin/clang+0x4d7fcd0) #33 0x0000000004cfacc0 clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) (/tmp/_update_lc/t/bin/clang+0x4cfacc0) #34 0x0000000004d28f2d clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, clang::DeclaratorContext, clang::SourceLocation*, clang::Parser::ForRangeInit*) (/tmp/_update_lc/t/bin/clang+0x4d28f2d) #35 0x0000000004cf9f32 clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) (/tmp/_update_lc/t/bin/clang+0x4cf9f32) #36 0x0000000004cf9938 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) (/tmp/_update_lc/t/bin/clang+0x4cf9938) #37 0x0000000004cf86fc clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (/tmp/_update_lc/t/bin/clang+0x4cf86fc) #38 0x0000000004cf6858 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, bool) (/tmp/_update_lc/t/bin/clang+0x4cf6858) #39 0x0000000004cf16ed clang::ParseAST(clang::Sema&, bool, bool) (/tmp/_update_lc/t/bin/clang+0x4cf16ed) #40 0x0000000003e3eb21 clang::FrontendAction::Execute() (/tmp/_update_lc/t/bin/clang+0x3e3eb21) #41 0x0000000003dba0e3 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/tmp/_update_lc/t/bin/clang+0x3dba0e3) #42 0x0000000003ee796b clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/tmp/_update_lc/t/bin/clang+0x3ee796b) #43 0x0000000002244636 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/tmp/_update_lc/t/bin/clang+0x2244636) #44 0x000000000224297d ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) (/tmp/_update_lc/t/bin/clang+0x224297d) #45 0x0000000002242619 main (/tmp/_update_lc/t/bin/clang+0x2242619) #46 0x00007ffff7b28042 __libc_start_main (/lib64/libc.so.6+0x27042) #47 0x000000000223f8ce _start (/tmp/_update_lc/t/bin/clang+0x223f8ce) /tmp/_update_lc/t/tools/clang/test/CXX/class/class.compare/class.eq/Output/p2.cpp.script: line 1: 4146047 Aborted /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem /tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify /home/dave/s/lp/clang/test/CXX/class/class.compare/class.eq/p2.cpp -- ******************** Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. ******************** Failed Tests (2): Clang :: CXX/class/class.compare/class.eq/p2.cpp Clang :: CXX/class/class.compare/class.spaceship/p1.cpp Testing Time: 117.51s Unsupported : 12906 Passed : 51214 Expectedly Failed: 100 Failed : 2 FAILED: CMakeFiles/check-all cd /tmp/_update_lc/t && /usr/bin/python3.8 /tmp/_update_lc/t/./bin/llvm-lit -sv --param USE_Z3_SOLVER=0 /tmp/_update_lc/t/tools/clang/test /tmp/_update_lc/t/tools/lld/test /tmp/_update_lc/t/tools/lldb/test /tmp/_update_lc/t/utils/lit /tmp/_update_lc/t/test ninja: build stopped: subcommand failed. + do_error 'FAILURE -- STAGE TWO BUILD of LLVM' 12 + echo FAILURE -- STAGE TWO BUILD of LLVM FAILURE -- STAGE TWO BUILD of LLVM + exit 12
If we're not going to try to do TRE at all on calls not marked "tail", we can probably drop this check.