diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -2185,12 +2185,6 @@ /// Clear execution profile of the function. void clearProfile(); - /// Converts conditional tail calls to unconditional tail calls. We do this to - /// handle conditional tail calls correctly and to give a chance to the - /// simplify conditional tail call pass to decide whether to re-optimize them - /// using profile information. - void removeConditionalTailCalls(); - // Convert COUNT_NO_PROFILE to 0 void removeTagsFromProfile(); diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -2204,10 +2204,6 @@ void BinaryFunction::postProcessCFG() { if (isSimple() && !BasicBlocks.empty()) { - // Convert conditional tail call branches to conditional branches that jump - // to a tail call. - removeConditionalTailCalls(); - postProcessProfile(); // Eliminate inconsistencies between branch instructions and CFG. @@ -2269,77 +2265,6 @@ } } -void BinaryFunction::removeConditionalTailCalls() { - // Blocks to be appended at the end. - std::vector> NewBlocks; - - for (auto BBI = begin(); BBI != end(); ++BBI) { - BinaryBasicBlock &BB = *BBI; - MCInst *CTCInstr = BB.getLastNonPseudoInstr(); - if (!CTCInstr) - continue; - - Optional TargetAddressOrNone = - BC.MIB->getConditionalTailCall(*CTCInstr); - if (!TargetAddressOrNone) - continue; - - // Gather all necessary information about CTC instruction before - // annotations are destroyed. - const int32_t CFIStateBeforeCTC = BB.getCFIStateAtInstr(CTCInstr); - uint64_t CTCTakenCount = BinaryBasicBlock::COUNT_NO_PROFILE; - uint64_t CTCMispredCount = BinaryBasicBlock::COUNT_NO_PROFILE; - if (hasValidProfile()) { - CTCTakenCount = BC.MIB->getAnnotationWithDefault( - *CTCInstr, "CTCTakenCount"); - CTCMispredCount = BC.MIB->getAnnotationWithDefault( - *CTCInstr, "CTCMispredCount"); - } - - // Assert that the tail call does not throw. - assert(!BC.MIB->getEHInfo(*CTCInstr) && - "found tail call with associated landing pad"); - - // Create a basic block with an unconditional tail call instruction using - // the same destination. - const MCSymbol *CTCTargetLabel = BC.MIB->getTargetSymbol(*CTCInstr); - assert(CTCTargetLabel && "symbol expected for conditional tail call"); - MCInst TailCallInstr; - BC.MIB->createTailCall(TailCallInstr, CTCTargetLabel, BC.Ctx.get()); - // Link new BBs to the original input offset of the BB where the CTC - // is, so we can map samples recorded in new BBs back to the original BB - // seem in the input binary (if using BAT) - std::unique_ptr TailCallBB = createBasicBlock( - BB.getInputOffset(), BC.Ctx->createNamedTempSymbol("TC")); - TailCallBB->addInstruction(TailCallInstr); - TailCallBB->setCFIState(CFIStateBeforeCTC); - - // Add CFG edge with profile info from BB to TailCallBB. - BB.addSuccessor(TailCallBB.get(), CTCTakenCount, CTCMispredCount); - - // Add execution count for the block. - TailCallBB->setExecutionCount(CTCTakenCount); - - BC.MIB->convertTailCallToJmp(*CTCInstr); - - BC.MIB->replaceBranchTarget(*CTCInstr, TailCallBB->getLabel(), - BC.Ctx.get()); - - // Add basic block to the list that will be added to the end. - NewBlocks.emplace_back(std::move(TailCallBB)); - - // Swap edges as the TailCallBB corresponds to the taken branch. - BB.swapConditionalSuccessors(); - - // This branch is no longer a conditional tail call. - BC.MIB->unsetConditionalTailCall(*CTCInstr); - } - - insertBasicBlocks(std::prev(end()), std::move(NewBlocks), - /* UpdateLayout */ true, - /* UpdateCFIState */ false); -} - uint64_t BinaryFunction::getFunctionScore() const { if (FunctionScore != -1) return FunctionScore; @@ -3362,7 +3287,8 @@ auto LastInstrRI = BB->getLastNonPseudo(); if (BB->succ_size() == 1) { if (LastInstrRI != BB->rend() && - BC.MIB->isConditionalBranch(*LastInstrRI)) { + BC.MIB->isConditionalBranch(*LastInstrRI) && + !BC.MIB->getConditionalTailCall(*LastInstrRI)) { // __builtin_unreachable() could create a conditional branch that // falls-through into the next function - hence the block will have only // one valid successor. Such behaviour is undefined and thus we remove diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp --- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp +++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp @@ -1953,20 +1953,16 @@ --I; // Ignore nops and CFIs - if (isPseudo(*I)) + if (isPseudo(*I) || isNoop(*I)) continue; // Stop when we find the first non-terminator - if (!isTerminator(*I)) - break; - - if (!isBranch(*I)) + if (!isTerminator(*I) || isTailCall(*I) || !isBranch(*I)) break; // Handle unconditional branches. if ((I->getOpcode() == X86::JMP_1 || I->getOpcode() == X86::JMP_2 || - I->getOpcode() == X86::JMP_4) && - !isTailCall(*I)) { + I->getOpcode() == X86::JMP_4)) { // If any code was seen after this unconditional branch, we've seen // unreachable code. Ignore them. CondBranch = nullptr;