https://github.com/ankurrj7 updated https://github.com/llvm/llvm-project/pull/201079
>From 09bb8b4e9cfc7e2090a71acf16bc72c68b2afb95 Mon Sep 17 00:00:00 2001 From: "[email protected]" <[email protected]> Date: Sat, 30 May 2026 15:51:58 +0000 Subject: [PATCH 1/5] [clang][coverage] Add opt-in call continuation counters --- clang/include/clang/Basic/CodeGenOptions.def | 3 + clang/include/clang/Options/Options.td | 7 + clang/lib/CodeGen/CGExpr.cpp | 62 +++++---- clang/lib/CodeGen/CodeGenFunction.h | 1 + clang/lib/CodeGen/CodeGenPGO.cpp | 103 ++++++++++++-- clang/lib/CodeGen/CodeGenPGO.h | 3 + clang/lib/CodeGen/CoverageMappingGen.cpp | 127 +++++++++++------- clang/lib/CodeGen/CoverageMappingGen.h | 20 ++- clang/lib/Driver/ToolChains/Clang.cpp | 22 ++- clang/lib/Driver/ToolChains/SYCL.cpp | 5 +- .../test/CoverageMapping/call-continuations.c | 32 +++++ .../test/Driver/coverage-call-continuations.c | 7 + 12 files changed, 298 insertions(+), 94 deletions(-) create mode 100644 clang/test/CoverageMapping/call-continuations.c create mode 100644 clang/test/Driver/coverage-call-continuations.c diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index 49374efbeb28b..66b458d0b0d87 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -250,6 +250,9 @@ CODEGENOPT(CoverageMapping , 1, 0, Benign) ///< Generate coverage mapping region ///< enable code coverage analysis. CODEGENOPT(DumpCoverageMapping , 1, 0, Benign) ///< Dump the generated coverage mapping ///< regions. +CODEGENOPT(CoverageCallContinuations , 1, 0, Benign) ///< Track coverage after + ///< calls with returned + ///< from call counters. CODEGENOPT(MCDCCoverage , 1, 0, Benign) ///< Enable MC/DC code coverage criteria. VALUE_CODEGENOPT(MCDCMaxConds, 16, 32767, Benign) ///< MC/DC Maximum conditions. VALUE_CODEGENOPT(MCDCMaxTVs, 32, 0x7FFFFFFE, Benign) ///< MC/DC Maximum test vectors. diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 5d5d409ac5494..6b6e6fffca071 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -1869,6 +1869,13 @@ defm coverage_mapping : BoolFOption<"coverage-mapping", "Generate coverage mapping to enable code coverage analysis">, NegFlag<SetFalse, [], [ClangOption], "Disable code coverage analysis">, BothFlags< [], [ClangOption, CLOption]>>; +defm coverage_call_continuations : BoolFOption<"coverage-call-continuations", + CodeGenOpts<"CoverageCallContinuations">, DefaultFalse, + PosFlag<SetTrue, [], [ClangOption, CC1Option], + "Track coverage after calls with returned-from-call counters">, + NegFlag<SetFalse, [], [ClangOption], + "Disable returned-from-call coverage counters">, + BothFlags<[], [ClangOption, CLOption]>>; defm mcdc_coverage : BoolFOption<"coverage-mcdc", CodeGenOpts<"MCDCCoverage">, DefaultFalse, PosFlag<SetTrue, [], [ClangOption, CC1Option], diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 6abe4331db552..1a468983cdb9b 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -6466,37 +6466,43 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E, } }); - // Builtins never have block type. - if (E->getCallee()->getType()->isBlockPointerType()) - return EmitBlockCallExpr(E, ReturnValue, CallOrInvoke); - - if (const auto *CE = dyn_cast<CXXMemberCallExpr>(E)) - return EmitCXXMemberCallExpr(CE, ReturnValue, CallOrInvoke); - - if (const auto *CE = dyn_cast<CUDAKernelCallExpr>(E)) - return EmitCUDAKernelCallExpr(CE, ReturnValue, CallOrInvoke); - - // A CXXOperatorCallExpr is created even for explicit object methods, but - // these should be treated like static function call. - if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(E)) - if (const auto *MD = - dyn_cast_if_present<CXXMethodDecl>(CE->getCalleeDecl()); - MD && MD->isImplicitObjectMemberFunction()) - return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue, CallOrInvoke); - - CGCallee callee = EmitCallee(E->getCallee()); + auto EmitTheCall = [&]() -> RValue { + // Builtins never have block type. + if (E->getCallee()->getType()->isBlockPointerType()) + return EmitBlockCallExpr(E, ReturnValue, CallOrInvoke); + + if (const auto *CE = dyn_cast<CXXMemberCallExpr>(E)) + return EmitCXXMemberCallExpr(CE, ReturnValue, CallOrInvoke); + + if (const auto *CE = dyn_cast<CUDAKernelCallExpr>(E)) + return EmitCUDAKernelCallExpr(CE, ReturnValue, CallOrInvoke); + + // A CXXOperatorCallExpr is created even for explicit object methods, but + // these should be treated like static function call. + if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(E)) + if (const auto *MD = + dyn_cast_if_present<CXXMethodDecl>(CE->getCalleeDecl()); + MD && MD->isImplicitObjectMemberFunction()) + return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue, CallOrInvoke); + + CGCallee callee = EmitCallee(E->getCallee()); + + if (callee.isBuiltin()) { + return EmitBuiltinExpr(callee.getBuiltinDecl(), callee.getBuiltinID(), E, + ReturnValue); + } - if (callee.isBuiltin()) { - return EmitBuiltinExpr(callee.getBuiltinDecl(), callee.getBuiltinID(), - E, ReturnValue); - } + if (callee.isPseudoDestructor()) { + return EmitCXXPseudoDestructorExpr(callee.getPseudoDestructorExpr()); + } - if (callee.isPseudoDestructor()) { - return EmitCXXPseudoDestructorExpr(callee.getPseudoDestructorExpr()); - } + return EmitCall(E->getCallee()->getType(), callee, E, ReturnValue, + /*Chain=*/nullptr, CallOrInvoke); + }; - return EmitCall(E->getCallee()->getType(), callee, E, ReturnValue, - /*Chain=*/nullptr, CallOrInvoke); + RValue RV = EmitTheCall(); + incrementCallContinuationProfileCounter(E); + return RV; } /// Emit a CallExpr without considering whether it might be a subclass. diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 6d0718c243812..aef340fe496f8 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1700,6 +1700,7 @@ class CodeGenFunction : public CodeGenTypeCache { void incrementProfileCounter(CounterForIncrement ExecSkip, const Stmt *S, bool UseBoth = false, llvm::Value *StepV = nullptr); + void incrementCallContinuationProfileCounter(const Stmt *S); bool isMCDCCoverageEnabled() const { return (CGM.getCodeGenOpts().hasProfileClangInstr() && diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 59faa3aef2460..989ede7e26222 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -17,6 +17,7 @@ #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/StmtVisitor.h" #include "clang/Basic/DiagnosticFrontend.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/MDBuilder.h" #include "llvm/Support/CommandLine.h" @@ -169,21 +170,31 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { PGOHash Hash; /// The map of statements to counters. llvm::DenseMap<const Stmt *, CounterPair> &CounterMap; + /// The map of calls to counters reached only when the call returns. + llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap; + /// Calls that need continuation counters assigned after normal counters. + SmallVector<const CallExpr *, 8> CallContinuations; /// The state of MC/DC Coverage in this function. MCDC::State &MCDCState; /// Maximum number of supported MC/DC conditions in a boolean expression. unsigned MCDCMaxCond; + /// Whether call-continuation coverage counters are enabled. + bool CoverageCallContinuations; /// The profile version. uint64_t ProfileVersion; /// Diagnostics Engine used to report warnings. DiagnosticsEngine &Diag; - MapRegionCounters(PGOHashVersion HashVersion, uint64_t ProfileVersion, - llvm::DenseMap<const Stmt *, CounterPair> &CounterMap, - MCDC::State &MCDCState, unsigned MCDCMaxCond, - DiagnosticsEngine &Diag) + MapRegionCounters( + PGOHashVersion HashVersion, uint64_t ProfileVersion, + llvm::DenseMap<const Stmt *, CounterPair> &CounterMap, + llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap, + MCDC::State &MCDCState, unsigned MCDCMaxCond, + bool CoverageCallContinuations, DiagnosticsEngine &Diag) : NextCounter(0), Hash(HashVersion), CounterMap(CounterMap), + CallContinuationCounterMap(CallContinuationCounterMap), MCDCState(MCDCState), MCDCMaxCond(MCDCMaxCond), + CoverageCallContinuations(CoverageCallContinuations), ProfileVersion(ProfileVersion), Diag(Diag) {} // Blocks and lambdas are handled as separate functions, so we need not @@ -343,6 +354,29 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { return Base::VisitBinaryOperator(S); } + static bool isNoReturnCall(const CallExpr *E) { + QualType CalleeType = E->getCallee()->getType(); + return getFunctionExtInfo(*CalleeType).getNoReturn(); + } + + /// Calls optionally get a continuation counter which is reached only if the + /// call returns normally. This lets coverage mapping distinguish a call being + /// reached from later source being reached. + bool VisitCallExpr(CallExpr *S) { + if (CoverageCallContinuations && CallContinuationCounterMap && + !isNoReturnCall(S)) + CallContinuations.push_back(S); + return Base::VisitCallExpr(S); + } + + void assignCallContinuationCounters() { + if (!CallContinuationCounterMap) + return; + + for (const CallExpr *S : CallContinuations) + (*CallContinuationCounterMap)[S] = NextCounter++; + } + /// Include \p S in the function hash. bool VisitStmt(Stmt *S) { auto Type = updateCounterMappings(S); @@ -992,11 +1026,21 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) { unsigned MCDCMaxConditions = (CGM.getCodeGenOpts().MCDCCoverage ? CGM.getCodeGenOpts().MCDCMaxConds : 0); + bool CoverageCallContinuations = + CGM.getCodeGenOpts().CoverageMapping && + CGM.getCodeGenOpts().CoverageCallContinuations; RegionCounterMap.reset(new llvm::DenseMap<const Stmt *, CounterPair>); + if (CoverageCallContinuations) + CallContinuationCounterMap.reset( + new llvm::DenseMap<const Stmt *, unsigned>); + else + CallContinuationCounterMap.reset(); RegionMCDCState.reset(new MCDC::State); MapRegionCounters Walker(HashVersion, ProfileVersion, *RegionCounterMap, - *RegionMCDCState, MCDCMaxConditions, CGM.getDiags()); + CallContinuationCounterMap.get(), *RegionMCDCState, + MCDCMaxConditions, CoverageCallContinuations, + CGM.getDiags()); if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) Walker.TraverseDecl(const_cast<FunctionDecl *>(FD)); else if (const ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(D)) @@ -1005,6 +1049,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) { Walker.TraverseDecl(const_cast<BlockDecl *>(BD)); else if (const CapturedDecl *CD = dyn_cast_or_null<CapturedDecl>(D)) Walker.TraverseDecl(const_cast<CapturedDecl *>(CD)); + Walker.assignCallContinuationCounters(); assert(Walker.NextCounter > 0 && "no entry counter mapped for decl"); NumRegionCounters = Walker.NextCounter; FunctionHash = Walker.Hash.finalize(); @@ -1041,9 +1086,11 @@ void CodeGenPGO::emitCounterRegionMapping(const Decl *D) { std::string CoverageMapping; llvm::raw_string_ostream OS(CoverageMapping); RegionMCDCState->BranchByStmt.clear(); - CoverageMappingGen MappingGen( - *CGM.getCoverageMapping(), CGM.getContext().getSourceManager(), - CGM.getLangOpts(), RegionCounterMap.get(), RegionMCDCState.get()); + CoverageMappingGen MappingGen(*CGM.getCoverageMapping(), + CGM.getContext().getSourceManager(), + CGM.getLangOpts(), RegionCounterMap.get(), + CallContinuationCounterMap.get(), + RegionMCDCState.get(), NumRegionCounters); MappingGen.emitCounterMapping(D, OS); if (CoverageMapping.empty()) @@ -1057,6 +1104,9 @@ void CodeGenPGO::emitCounterRegionMapping(const Decl *D) { if (V.Skipped.hasValue()) MaxNumCounters = std::max(MaxNumCounters, V.Skipped + 1); } + if (CallContinuationCounterMap) + for (const auto &[_, V] : *CallContinuationCounterMap) + MaxNumCounters = std::max(MaxNumCounters, V + 1); NumRegionCounters = MaxNumCounters; CGM.getCoverageMapping()->addFunctionMappingRecord( @@ -1157,6 +1207,34 @@ void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, CGM.getIntrinsic(llvm::Intrinsic::instrprof_increment_step), Args); } +void CodeGenPGO::emitCallContinuationCounter(CGBuilderTy &Builder, + const Stmt *S) { + if (!CallContinuationCounterMap) + return; + + auto I = CallContinuationCounterMap->find(S); + if (I == CallContinuationCounterMap->end()) + return; + + if (!Builder.GetInsertBlock()) + return; + + auto *NormalizedFuncNameVarPtr = + llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast( + FuncNameVar, llvm::PointerType::get(CGM.getLLVMContext(), 0)); + + llvm::Value *Args[] = { + NormalizedFuncNameVarPtr, Builder.getInt64(FunctionHash), + Builder.getInt32(NumRegionCounters), Builder.getInt32(I->second)}; + + if (llvm::EnableSingleByteCoverage) + Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::instrprof_cover), + ArrayRef(Args, 4)); + else + Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::instrprof_increment), + ArrayRef(Args, 4)); +} + bool CodeGenPGO::canEmitMCDCCoverage(const CGBuilderTy &Builder) { return (CGM.getCodeGenOpts().hasProfileClangInstr() && CGM.getCodeGenOpts().MCDCCoverage && Builder.GetInsertBlock()); @@ -1487,6 +1565,15 @@ void CodeGenFunction::incrementProfileCounter(CounterForIncrement ExecSkip, PGO->setCurrentStmt(S); } +void CodeGenFunction::incrementCallContinuationProfileCounter(const Stmt *S) { + if (CGM.getCodeGenOpts().hasProfileClangInstr() && + !CurFn->hasFnAttribute(llvm::Attribute::NoProfile) && + !CurFn->hasFnAttribute(llvm::Attribute::SkipProfile)) { + auto AL = ApplyDebugLocation::CreateArtificial(*this); + PGO->emitCallContinuationCounter(Builder, S); + } +} + bool CodeGenFunction::hasSkipCounter(const Stmt *S) const { return PGO->hasSkipCounter(S); } diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index e44574b5d209f..613ba8db4bdab 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -36,6 +36,8 @@ class CodeGenPGO { unsigned NumRegionCounters; uint64_t FunctionHash; std::unique_ptr<llvm::DenseMap<const Stmt *, CounterPair>> RegionCounterMap; + std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> + CallContinuationCounterMap; std::unique_ptr<llvm::DenseMap<const Stmt *, uint64_t>> StmtCountMap; std::unique_ptr<llvm::InstrProfRecord> ProfRecord; std::unique_ptr<MCDC::State> RegionMCDCState; @@ -128,6 +130,7 @@ class CodeGenPGO { void emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, bool UseFalsePath, bool UseBoth, llvm::Value *StepV); + void emitCallContinuationCounter(CGBuilderTy &Builder, const Stmt *S); void emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder, const Expr *S, CodeGenFunction &CGF); void emitMCDCParameters(CGBuilderTy &Builder); diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index c90afacbde293..1865a4f4d1cb1 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -914,6 +914,9 @@ struct CounterCoverageMappingBuilder /// The map of statements to count values. llvm::DenseMap<const Stmt *, CounterPair> &CounterMap; + /// The map of calls to counters reached only when the call returns. + llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap; + /// Used to expand an allocatd SkipCnt to Expression with known counters. /// Key: SkipCnt /// Val: Subtract Expression @@ -942,10 +945,11 @@ struct CounterCoverageMappingBuilder /// expressions cross file or macro boundaries. SourceLocation MostRecentLocation; - /// Whether the visitor at a terminate statement. - bool HasTerminateStmt = false; + /// Whether the visitor changed the active region and needs a gap region + /// before the next statement. + bool HasGapRegion = false; - /// Gap region counter after terminate statement. + /// Counter to use for the pending gap region. Counter GapRegionCounter; /// Return a counter for the subtraction of \c RHS from \c LHS @@ -972,6 +976,15 @@ struct CounterCoverageMappingBuilder return Counter::getCounter(CounterMap[S].Executed); } + std::optional<Counter> getCallContinuationCounter(const Stmt *S) { + if (!CallContinuationCounterMap) + return std::nullopt; + auto I = CallContinuationCounterMap->find(S); + if (I == CallContinuationCounterMap->end()) + return std::nullopt; + return Counter::getCounter(I->second); + } + struct BranchCounterPair { Counter Executed; ///< The Counter previously assigned. Counter Skipped; ///< An expression (Parent-Executed), or equivalent to it. @@ -1397,7 +1410,22 @@ struct CounterCoverageMappingBuilder if (!Region.hasEndLoc()) Region.setEndLoc(EndLoc); pushRegion(Counter::getZero()); - HasTerminateStmt = true; + HasGapRegion = true; + } + + void startCallContinuationRegion(const Stmt *S, Counter ContinuationCount) { + SourceMappingRegion &Region = getRegion(); + SourceLocation EndLoc = getEnd(S); + if (EndLoc.isInvalid()) + return; + + std::optional<SourceLocation> OriginalEndLoc = + Region.hasEndLoc() ? std::optional<SourceLocation>(Region.getEndLoc()) + : std::nullopt; + Region.setEndLoc(EndLoc); + pushRegion(ContinuationCount, std::nullopt, OriginalEndLoc); + GapRegionCounter = ContinuationCount; + HasGapRegion = true; } /// Find a valid gap range between \p AfterLoc and \p BeforeLoc. @@ -1539,9 +1567,12 @@ struct CounterCoverageMappingBuilder CounterCoverageMappingBuilder( CoverageMappingModuleGen &CVM, llvm::DenseMap<const Stmt *, CounterPair> &CounterMap, - MCDC::State &MCDCState, SourceManager &SM, const LangOptions &LangOpts) + llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap, + MCDC::State &MCDCState, SourceManager &SM, const LangOptions &LangOpts, + unsigned NextCounter) : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap), - NextCounterNum(CounterMap.size()), MCDCState(MCDCState), + CallContinuationCounterMap(CallContinuationCounterMap), + NextCounterNum(NextCounter), MCDCState(MCDCState), MCDCBuilder(CVM.getCodeGenModule(), MCDCState) {} /// Write the mapping data to the output stream @@ -1564,35 +1595,34 @@ struct CounterCoverageMappingBuilder if (S->getBeginLoc().isValid()) extendRegion(S); const Stmt *LastStmt = nullptr; - bool SaveTerminateStmt = HasTerminateStmt; - HasTerminateStmt = false; + bool SaveGapRegion = HasGapRegion; + HasGapRegion = false; GapRegionCounter = Counter::getZero(); for (const Stmt *Child : S->children()) if (Child) { - // If last statement contains terminate statements, add a gap area - // between the two statements. - if (LastStmt && HasTerminateStmt) { + // If the last statement changed the active region, add a gap area + // between the two statements with the new counter. + if (LastStmt && HasGapRegion) { auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child)); if (Gap) fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), GapRegionCounter); - SaveTerminateStmt = true; - HasTerminateStmt = false; + SaveGapRegion = true; + HasGapRegion = false; } this->Visit(Child); LastStmt = Child; } - if (SaveTerminateStmt) - HasTerminateStmt = true; + if (SaveGapRegion) + HasGapRegion = true; handleFileExit(getEnd(S)); } void VisitStmtExpr(const StmtExpr *E) { Visit(E->getSubStmt()); - // Any region termination (such as a noreturn CallExpr) within the statement - // expression has been handled by visiting the sub-statement. The visitor - // cannot be at a terminate statement leaving the statement expression. - HasTerminateStmt = false; + // Any region transition within the statement expression has been handled by + // visiting the sub-statement. It cannot need a gap outside the expression. + HasGapRegion = false; } void VisitDecl(const Decl *D) { @@ -1692,6 +1722,9 @@ struct CounterCoverageMappingBuilder QualType CalleeType = E->getCallee()->getType(); if (getFunctionExtInfo(*CalleeType).getNoReturn()) terminateRegion(E); + else if (std::optional<Counter> ContinuationCounter = + getCallContinuationCounter(E)) + startCallContinuationRegion(E, *ContinuationCounter); } void VisitWhileStmt(const WhileStmt *S) { @@ -1706,8 +1739,8 @@ struct CounterCoverageMappingBuilder Counter BackedgeCount = propagateCounts(BodyCount, S->getBody()); BreakContinue BC = BreakContinueStack.pop_back_val(); - bool BodyHasTerminateStmt = HasTerminateStmt; - HasTerminateStmt = false; + bool BodyHasGapRegion = HasGapRegion; + HasGapRegion = false; // Go back to handle the condition. Counter CondCount = @@ -1727,8 +1760,8 @@ struct CounterCoverageMappingBuilder if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; - if (BodyHasTerminateStmt) - HasTerminateStmt = true; + if (BodyHasGapRegion) + HasGapRegion = true; } // Create Branch Region around condition. @@ -1749,8 +1782,8 @@ struct CounterCoverageMappingBuilder BreakContinue BC = BreakContinueStack.pop_back_val(); - bool BodyHasTerminateStmt = HasTerminateStmt; - HasTerminateStmt = false; + bool BodyHasGapRegion = HasGapRegion; + HasGapRegion = false; Counter CondCount = addCounters(BackedgeCount, BC.ContinueCount); auto BranchCount = getBranchCounterPair(S, CondCount); @@ -1762,8 +1795,8 @@ struct CounterCoverageMappingBuilder if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; - if (BodyHasTerminateStmt) - HasTerminateStmt = true; + if (BodyHasGapRegion) + HasGapRegion = true; } // Create Branch Region around condition. @@ -1788,8 +1821,8 @@ struct CounterCoverageMappingBuilder Counter BackedgeCount = propagateCounts(BodyCount, S->getBody()); BreakContinue BodyBC = BreakContinueStack.pop_back_val(); - bool BodyHasTerminateStmt = HasTerminateStmt; - HasTerminateStmt = false; + bool BodyHasGapRegion = HasGapRegion; + HasGapRegion = false; // The increment is essentially part of the body but it needs to include // the count for all the continue statements. @@ -1821,8 +1854,8 @@ struct CounterCoverageMappingBuilder if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; - if (BodyHasTerminateStmt) - HasTerminateStmt = true; + if (BodyHasGapRegion) + HasGapRegion = true; } // Create Branch Region around condition. @@ -1844,8 +1877,8 @@ struct CounterCoverageMappingBuilder Counter BackedgeCount = propagateCounts(BodyCount, S->getBody()); BreakContinue BC = BreakContinueStack.pop_back_val(); - bool BodyHasTerminateStmt = HasTerminateStmt; - HasTerminateStmt = false; + bool BodyHasGapRegion = HasGapRegion; + HasGapRegion = false; // The body count applies to the area immediately after the range. auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getBody())); @@ -1861,8 +1894,8 @@ struct CounterCoverageMappingBuilder if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; - if (BodyHasTerminateStmt) - HasTerminateStmt = true; + if (BodyHasGapRegion) + HasGapRegion = true; } // Create Branch Region around condition. @@ -2085,12 +2118,13 @@ struct CounterCoverageMappingBuilder // needed to handle macros that generate the "if" but not the condition. extendRegion(S->getCond()); - Counter ParentCount = getRegion().getCounter(); - auto [ThenCount, ElseCount] = getBranchCounterPair(S, ParentCount); - // Emitting a counter for the condition makes it easier to interpret the // counter for the body when looking at the coverage. - propagateCounts(ParentCount, S->getCond()); + Counter ParentCount = getRegion().getCounter(); + Counter CondExitCount = propagateCounts(ParentCount, S->getCond()); + Counter BranchParentCount = + CallContinuationCounterMap ? CondExitCount : ParentCount; + auto [ThenCount, ElseCount] = getBranchCounterPair(S, BranchParentCount); // The 'then' count applies to the area immediately after the condition. std::optional<SourceRange> Gap = @@ -2102,8 +2136,8 @@ struct CounterCoverageMappingBuilder Counter OutCount = propagateCounts(ThenCount, S->getThen()); if (const Stmt *Else = S->getElse()) { - bool ThenHasTerminateStmt = HasTerminateStmt; - HasTerminateStmt = false; + bool ThenHasGapRegion = HasGapRegion; + HasGapRegion = false; // The 'else' count applies to the area immediately after the 'then'. std::optional<SourceRange> Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else)); @@ -2113,12 +2147,12 @@ struct CounterCoverageMappingBuilder OutCount = addCounters(OutCount, propagateCounts(ElseCount, Else)); - if (ThenHasTerminateStmt) - HasTerminateStmt = true; + if (ThenHasGapRegion) + HasGapRegion = true; } else OutCount = addCounters(OutCount, ElseCount); - if (!IsCounterEqual(OutCount, ParentCount)) { + if (!IsCounterEqual(OutCount, BranchParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; } @@ -2682,8 +2716,9 @@ unsigned CoverageMappingModuleGen::getFileID(FileEntryRef File) { void CoverageMappingGen::emitCounterMapping(const Decl *D, llvm::raw_ostream &OS) { assert(CounterMap && MCDCState); - CounterCoverageMappingBuilder Walker(CVM, *CounterMap, *MCDCState, SM, - LangOpts); + CounterCoverageMappingBuilder Walker(CVM, *CounterMap, + CallContinuationCounterMap, *MCDCState, + SM, LangOpts, NextCounter); Walker.VisitDecl(D); Walker.write(OS); } diff --git a/clang/lib/CodeGen/CoverageMappingGen.h b/clang/lib/CodeGen/CoverageMappingGen.h index 0ed50597e1dc3..15fe381206830 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.h +++ b/clang/lib/CodeGen/CoverageMappingGen.h @@ -160,20 +160,26 @@ class CoverageMappingGen { SourceManager &SM; const LangOptions &LangOpts; llvm::DenseMap<const Stmt *, CounterPair> *CounterMap; + llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap; MCDC::State *MCDCState; + unsigned NextCounter; public: CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM, const LangOptions &LangOpts) : CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(nullptr), - MCDCState(nullptr) {} - - CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM, - const LangOptions &LangOpts, - llvm::DenseMap<const Stmt *, CounterPair> *CounterMap, - MCDC::State *MCDCState) + CallContinuationCounterMap(nullptr), MCDCState(nullptr), + NextCounter(0) {} + + CoverageMappingGen( + CoverageMappingModuleGen &CVM, SourceManager &SM, + const LangOptions &LangOpts, + llvm::DenseMap<const Stmt *, CounterPair> *CounterMap, + llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap, + MCDC::State *MCDCState, unsigned NextCounter) : CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(CounterMap), - MCDCState(MCDCState) {} + CallContinuationCounterMap(CallContinuationCounterMap), + MCDCState(MCDCState), NextCounter(NextCounter) {} /// Emit the coverage mapping data which maps the regions of /// code to counters that will be used to find the execution diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index a3a3954bc464e..f8a9d86eac1a1 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -542,8 +542,9 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C, Args.hasArg(options::OPT_coverage); bool EmitCovData = TC.needsGCovInstrumentation(Args); - if (Args.hasFlag(options::OPT_fcoverage_mapping, - options::OPT_fno_coverage_mapping, false)) { + bool CoverageMappingEnabled = Args.hasFlag( + options::OPT_fcoverage_mapping, options::OPT_fno_coverage_mapping, false); + if (CoverageMappingEnabled) { if (!ProfileGenerateArg) D.Diag(clang::diag::err_drv_argument_only_allowed_with) << "-fcoverage-mapping" @@ -552,10 +553,19 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C, CmdArgs.push_back("-fcoverage-mapping"); } + if (Args.hasFlag(options::OPT_fcoverage_call_continuations, + options::OPT_fno_coverage_call_continuations, false)) { + if (!CoverageMappingEnabled) + D.Diag(clang::diag::err_drv_argument_only_allowed_with) + << "-fcoverage-call-continuations" + << "-fcoverage-mapping"; + + CmdArgs.push_back("-fcoverage-call-continuations"); + } + if (Args.hasFlag(options::OPT_fmcdc_coverage, options::OPT_fno_mcdc_coverage, false)) { - if (!Args.hasFlag(options::OPT_fcoverage_mapping, - options::OPT_fno_coverage_mapping, false)) + if (!CoverageMappingEnabled) D.Diag(clang::diag::err_drv_argument_only_allowed_with) << "-fcoverage-mcdc" << "-fcoverage-mapping"; @@ -9705,6 +9715,8 @@ static bool requiresProfileRT(unsigned ID) { case options::OPT_fprofile_instr_generate_EQ: case options::OPT_fcoverage_mapping: case options::OPT_fno_coverage_mapping: + case options::OPT_fcoverage_call_continuations: + case options::OPT_fno_coverage_call_continuations: case options::OPT_fcoverage_compilation_dir_EQ: case options::OPT_ffile_compilation_dir_EQ: case options::OPT_fcoverage_prefix_map_EQ: @@ -9777,6 +9789,8 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA, OPT_fprofile_instr_generate_EQ, OPT_fcoverage_mapping, OPT_fno_coverage_mapping, + OPT_fcoverage_call_continuations, + OPT_fno_coverage_call_continuations, OPT_fcoverage_compilation_dir_EQ, OPT_ffile_compilation_dir_EQ, OPT_fcoverage_prefix_map_EQ, diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index d41b5c06da53f..8aedf0b8725f3 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -94,7 +94,10 @@ static ArrayRef<options::ID> getUnsupportedOpts() { options::OPT_fno_test_coverage, // -f[no-]test-coverage options::OPT_fcoverage_mapping, options::OPT_fno_coverage_mapping, // -f[no-]coverage-mapping - options::OPT_coverage, // --coverage + options::OPT_fcoverage_call_continuations, + options::OPT_fno_coverage_call_continuations, + // -f[no-]coverage-call-continuations + options::OPT_coverage, // --coverage options::OPT_fprofile_instr_generate, options::OPT_fprofile_instr_generate_EQ, options::OPT_fno_profile_instr_generate, // -f[no-]profile-instr-generate diff --git a/clang/test/CoverageMapping/call-continuations.c b/clang/test/CoverageMapping/call-continuations.c new file mode 100644 index 0000000000000..ef4d430267958 --- /dev/null +++ b/clang/test/CoverageMapping/call-continuations.c @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -dump-coverage-mapping -emit-llvm -o - %s | FileCheck %s --check-prefixes=MAP,IR +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -o - %s | FileCheck %s --check-prefix=NOCC + +void f(void); +__attribute__((returns_twice)) int returns_twice(void); + +int after_call(void) { + f(); + return 1; +} + +// IR-LABEL: define{{.*}} i32 @after_call( +// IR: call void @f() +// IR-NEXT: load i64, ptr getelementptr inbounds ([2 x i64], ptr @__profc_after_call, i32 0, i32 1) +// IR: ret i32 1 + +int setjmp_like(void) { + if (returns_twice() == 0) + return 1; + return 2; +} + +// IR-LABEL: define{{.*}} i32 @setjmp_like +// IR: call{{.*}}@returns_twice +// IR-NEXT: load i64, ptr getelementptr inbounds ([3 x i64], ptr @__profc_setjmp_like, i32 0, i32 2) +// MAP-LABEL: after_call: +// MAP: Gap,File 0, [[@LINE-19]]:7 -> [[@LINE-18]]:3 = #1 +// MAP: File 0, [[@LINE-19]]:3 -> [[@LINE-18]]:2 = #1 +// MAP-LABEL: setjmp_like: +// MAP: Branch,File 0, [[@LINE-12]]:7 -> [[@LINE-12]]:27 = #1, (#2 - #1) +// NOCC-LABEL: setjmp_like: +// NOCC: Branch,File 0, [[@LINE-14]]:7 -> [[@LINE-14]]:27 = #1, (#0 - #1) diff --git a/clang/test/Driver/coverage-call-continuations.c b/clang/test/Driver/coverage-call-continuations.c new file mode 100644 index 0000000000000..67b4053c60773 --- /dev/null +++ b/clang/test/Driver/coverage-call-continuations.c @@ -0,0 +1,7 @@ +// RUN: %clang -### -fprofile-instr-generate -fcoverage-mapping -fcoverage-call-continuations -c %s 2>&1 | FileCheck %s +// RUN: not %clang -### -fcoverage-call-continuations -c %s 2>&1 | FileCheck %s --check-prefix=ERR + +// CHECK: "-fcoverage-mapping" +// CHECK: "-fcoverage-call-continuations" + +// ERR: error: invalid argument '-fcoverage-call-continuations' only allowed with '-fcoverage-mapping' >From 977996005cb4aa087ef3f48c9b34560cfd23374e Mon Sep 17 00:00:00 2001 From: "[email protected]" <[email protected]> Date: Tue, 2 Jun 2026 09:48:22 +0000 Subject: [PATCH 2/5] [clang][coverage] Tighten call continuation counters --- clang/lib/CodeGen/CGExpr.cpp | 3 +- clang/lib/CodeGen/CodeGenPGO.cpp | 29 +++- clang/lib/CodeGen/CoverageMappingGen.cpp | 141 ++++++++++++------ .../call-continuations-tight.c | 69 +++++++++ 4 files changed, 193 insertions(+), 49 deletions(-) create mode 100644 clang/test/CoverageMapping/call-continuations-tight.c diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 1a468983cdb9b..b53fa43f4ecb0 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -6501,7 +6501,8 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E, }; RValue RV = EmitTheCall(); - incrementCallContinuationProfileCounter(E); + if (E != MustTailCall) + incrementCallContinuationProfileCounter(E); return RV; } diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 989ede7e26222..d5e41e2d13196 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -14,6 +14,7 @@ #include "CGDebugInfo.h" #include "CodeGenFunction.h" #include "CoverageMappingGen.h" +#include "clang/AST/Attr.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/StmtVisitor.h" #include "clang/Basic/DiagnosticFrontend.h" @@ -23,6 +24,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/Endian.h" #include "llvm/Support/MD5.h" +#include "llvm/Support/SaveAndRestore.h" #include <optional> namespace llvm { @@ -180,6 +182,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { unsigned MCDCMaxCond; /// Whether call-continuation coverage counters are enabled. bool CoverageCallContinuations; + /// The call currently marked as musttail, if any. + const CallExpr *MustTailCall = nullptr; /// The profile version. uint64_t ProfileVersion; /// Diagnostics Engine used to report warnings. @@ -208,6 +212,29 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { } bool TraverseCapturedStmt(CapturedStmt *CS) { return true; } + static const CallExpr *getMustTailCall(const AttributedStmt *S) { + for (const Attr *A : S->getAttrs()) { + if (A->getKind() != attr::MustTail) + continue; + + const auto *R = dyn_cast<ReturnStmt>(S->getSubStmt()); + if (!R || !R->getRetValue()) + return nullptr; + return dyn_cast<CallExpr>(R->getRetValue()->IgnoreParens()); + } + return nullptr; + } + + bool TraverseAttributedStmt(AttributedStmt *S) { + const CallExpr *NewMustTailCall = getMustTailCall(S); + if (!NewMustTailCall) + return Base::TraverseAttributedStmt(S); + + llvm::SaveAndRestore<const CallExpr *> SaveMustTail(MustTailCall, + NewMustTailCall); + return Base::TraverseAttributedStmt(S); + } + bool VisitDecl(const Decl *D) { switch (D->getKind()) { default: @@ -364,7 +391,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { /// reached from later source being reached. bool VisitCallExpr(CallExpr *S) { if (CoverageCallContinuations && CallContinuationCounterMap && - !isNoReturnCall(S)) + !isNoReturnCall(S) && S != MustTailCall) CallContinuations.push_back(S); return Base::VisitCallExpr(S); } diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 1865a4f4d1cb1..ac70667040727 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -1745,11 +1745,13 @@ struct CounterCoverageMappingBuilder // Go back to handle the condition. Counter CondCount = addCounters(ParentCount, BackedgeCount, BC.ContinueCount); - auto BranchCount = getBranchCounterPair(S, CondCount); - assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount); - propagateCounts(CondCount, S->getCond()); + Counter CondExitCount = propagateCounts(CondCount, S->getCond()); adjustForOutOfOrderTraversal(getEnd(S)); + Counter BranchParentCount = + CallContinuationCounterMap ? CondExitCount : CondCount; + auto BranchCount = getBranchCounterPair(S, BranchParentCount); + assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount); // The body count applies to the area immediately after the increment. auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getBody())); @@ -1786,10 +1788,12 @@ struct CounterCoverageMappingBuilder HasGapRegion = false; Counter CondCount = addCounters(BackedgeCount, BC.ContinueCount); - auto BranchCount = getBranchCounterPair(S, CondCount); - assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount); - propagateCounts(CondCount, S->getCond()); + Counter CondExitCount = propagateCounts(CondCount, S->getCond()); + Counter BranchParentCount = + CallContinuationCounterMap ? CondExitCount : CondCount; + auto BranchCount = getBranchCounterPair(S, BranchParentCount); + assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount); Counter OutCount = addCounters(BC.BreakCount, BranchCount.Skipped); if (!IsCounterEqual(OutCount, ParentCount)) { @@ -1836,13 +1840,16 @@ struct CounterCoverageMappingBuilder Counter CondCount = addCounters( addCounters(ParentCount, BackedgeCount, BodyBC.ContinueCount), IncrementBC.ContinueCount); - auto BranchCount = getBranchCounterPair(S, CondCount); - assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount); + Counter CondExitCount = CondCount; if (const Expr *Cond = S->getCond()) { - propagateCounts(CondCount, Cond); + CondExitCount = propagateCounts(CondCount, Cond); adjustForOutOfOrderTraversal(getEnd(S)); } + Counter BranchParentCount = + CallContinuationCounterMap ? CondExitCount : CondCount; + auto BranchCount = getBranchCounterPair(S, BranchParentCount); + assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount); // The body count applies to the area immediately after the increment. auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getBody())); @@ -2184,14 +2191,31 @@ struct CounterCoverageMappingBuilder extendRegion(E); Counter ParentCount = getRegion().getCounter(); - auto [TrueCount, FalseCount] = getBranchCounterPair(E, ParentCount); Counter OutCount; if (const auto *BCO = dyn_cast<BinaryConditionalOperator>(E)) { - propagateCounts(ParentCount, BCO->getCommon()); + Counter CondExitCount = propagateCounts(ParentCount, BCO->getCommon()); + Counter BranchParentCount = + CallContinuationCounterMap ? CondExitCount : ParentCount; + auto [TrueCount, FalseCount] = getBranchCounterPair(E, BranchParentCount); OutCount = TrueCount; + + extendRegion(E->getFalseExpr()); + OutCount = + addCounters(OutCount, propagateCounts(FalseCount, E->getFalseExpr())); + + if (!IsCounterEqual(OutCount, BranchParentCount)) { + pushRegion(OutCount); + GapRegionCounter = OutCount; + } + + createBranchRegion(E->getCond(), TrueCount, FalseCount); + return; } else { - propagateCounts(ParentCount, E->getCond()); + Counter CondExitCount = propagateCounts(ParentCount, E->getCond()); + Counter BranchParentCount = + CallContinuationCounterMap ? CondExitCount : ParentCount; + auto [TrueCount, FalseCount] = getBranchCounterPair(E, BranchParentCount); // The 'then' count applies to the area immediately after the condition. auto Gap = findGapAreaBetween(E->getQuestionLoc(), getStart(E->getTrueExpr())); @@ -2200,19 +2224,19 @@ struct CounterCoverageMappingBuilder extendRegion(E->getTrueExpr()); OutCount = propagateCounts(TrueCount, E->getTrueExpr()); - } - extendRegion(E->getFalseExpr()); - OutCount = - addCounters(OutCount, propagateCounts(FalseCount, E->getFalseExpr())); + extendRegion(E->getFalseExpr()); + OutCount = + addCounters(OutCount, propagateCounts(FalseCount, E->getFalseExpr())); - if (!IsCounterEqual(OutCount, ParentCount)) { - pushRegion(OutCount); - GapRegionCounter = OutCount; - } + if (!IsCounterEqual(OutCount, BranchParentCount)) { + pushRegion(OutCount); + GapRegionCounter = OutCount; + } - // Create Branch Region around condition. - createBranchRegion(E->getCond(), TrueCount, FalseCount); + // Create Branch Region around condition. + createBranchRegion(E->getCond(), TrueCount, FalseCount); + } } inline unsigned findMCDCBranchesInSourceRegion( @@ -2332,8 +2356,10 @@ struct CounterCoverageMappingBuilder CurCondIDs[true] = RHSid; auto DecisionLHS = CurCondIDs; + Counter ParentCnt = getRegion().getCounter(); + extendRegion(E->getLHS()); - propagateCounts(getRegion().getCounter(), E->getLHS()); + Counter LHSExitCnt = propagateCounts(ParentCnt, E->getLHS()); handleFileExit(getEnd(E->getLHS())); // Restore CurCondIDs. @@ -2349,25 +2375,35 @@ struct CounterCoverageMappingBuilder fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), getRegionCounter(E)); } + // Extract the RHS's Execution Counter. + Counter LHSBranchParentCnt = + CallContinuationCounterMap ? LHSExitCnt : getRegion().getCounter(); + auto [RHSExecCnt, LHSFalseCnt] = + getBranchCounterPair(E, LHSBranchParentCnt); + // Counter tracks the right hand side of a logical and operator. extendRegion(E->getRHS()); - propagateCounts(getRegionCounter(E), E->getRHS()); - - // Extract the Parent Region Counter. - Counter ParentCnt = getRegion().getCounter(); - - // Extract the RHS's Execution Counter. - auto [RHSExecCnt, LHSExitCnt] = getBranchCounterPair(E, ParentCnt); + Counter RHSExitCnt = propagateCounts(RHSExecCnt, E->getRHS()); // Extract the RHS's "True" Instance Counter. - auto [RHSTrueCnt, RHSExitCnt] = - getBranchCounterPair(E->getRHS(), RHSExecCnt); + Counter RHSBranchParentCnt = + CallContinuationCounterMap ? RHSExitCnt : RHSExecCnt; + auto [RHSTrueCnt, RHSFalseCnt] = + getBranchCounterPair(E->getRHS(), RHSBranchParentCnt); + + if (CallContinuationCounterMap) { + Counter OutCount = addCounters(LHSFalseCnt, RHSExitCnt); + if (!IsCounterEqual(OutCount, ParentCnt)) { + getRegion().setCounter(OutCount); + GapRegionCounter = OutCount; + } + } // Create Branch Region around LHS condition. - createBranchRegion(E->getLHS(), RHSExecCnt, LHSExitCnt, DecisionLHS); + createBranchRegion(E->getLHS(), RHSExecCnt, LHSFalseCnt, DecisionLHS); // Create Branch Region around RHS condition. - createBranchRegion(E->getRHS(), RHSTrueCnt, RHSExitCnt, DecisionRHS); + createBranchRegion(E->getRHS(), RHSTrueCnt, RHSFalseCnt, DecisionRHS); // Create MCDC Decision Region when E is at the top level. createOrCancelDecision(E, SourceRegionsSince); @@ -2400,8 +2436,10 @@ struct CounterCoverageMappingBuilder CurCondIDs[false] = RHSid; auto DecisionLHS = CurCondIDs; + Counter ParentCnt = getRegion().getCounter(); + extendRegion(E->getLHS()); - Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS()); + Counter LHSExitCnt = propagateCounts(ParentCnt, E->getLHS()); handleFileExit(getEnd(E->getLHS())); // Track LHS True/False Decision. @@ -2417,29 +2455,38 @@ struct CounterCoverageMappingBuilder fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), getRegionCounter(E)); } + // Extract the RHS's Execution Counter. + Counter LHSBranchParentCnt = + CallContinuationCounterMap ? LHSExitCnt : getRegion().getCounter(); + auto [RHSExecCnt, LHSTrueCnt] = getBranchCounterPair(E, LHSBranchParentCnt); + // Counter tracks the right hand side of a logical or operator. extendRegion(E->getRHS()); - propagateCounts(getRegionCounter(E), E->getRHS()); - - // Extract the Parent Region Counter. - Counter ParentCnt = getRegion().getCounter(); - - // Extract the RHS's Execution Counter. - auto [RHSExecCnt, LHSExitCnt] = getBranchCounterPair(E, ParentCnt); + Counter RHSExitCnt = propagateCounts(RHSExecCnt, E->getRHS()); // Extract the RHS's "False" Instance Counter. - auto [RHSFalseCnt, RHSExitCnt] = - getBranchCounterPair(E->getRHS(), RHSExecCnt); + Counter RHSBranchParentCnt = + CallContinuationCounterMap ? RHSExitCnt : RHSExecCnt; + auto [RHSFalseCnt, RHSTrueCnt] = + getBranchCounterPair(E->getRHS(), RHSBranchParentCnt); if (!shouldVisitRHS(E->getLHS())) { - GapRegionCounter = OutCount; + GapRegionCounter = LHSExitCnt; + } + + if (CallContinuationCounterMap) { + Counter OutCount = addCounters(LHSTrueCnt, RHSExitCnt); + if (!IsCounterEqual(OutCount, ParentCnt)) { + getRegion().setCounter(OutCount); + GapRegionCounter = OutCount; + } } // Create Branch Region around LHS condition. - createBranchRegion(E->getLHS(), LHSExitCnt, RHSExecCnt, DecisionLHS); + createBranchRegion(E->getLHS(), LHSTrueCnt, RHSExecCnt, DecisionLHS); // Create Branch Region around RHS condition. - createBranchRegion(E->getRHS(), RHSExitCnt, RHSFalseCnt, DecisionRHS); + createBranchRegion(E->getRHS(), RHSTrueCnt, RHSFalseCnt, DecisionRHS); // Create MCDC Decision Region when E is at the top level. createOrCancelDecision(E, SourceRegionsSince); diff --git a/clang/test/CoverageMapping/call-continuations-tight.c b/clang/test/CoverageMapping/call-continuations-tight.c new file mode 100644 index 0000000000000..11b01870cf620 --- /dev/null +++ b/clang/test/CoverageMapping/call-continuations-tight.c @@ -0,0 +1,69 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -dump-coverage-mapping -emit-llvm -o - %s | FileCheck %s --check-prefixes=IR,MAP + +int printf(const char *, ...); +void f(void); +int g(void); +__attribute__((returns_twice)) int returns_twice(void); +int tail_callee(int); + +int block_after_call(int argc) { + { + if (argc > 2) + return 2; + printf("one\n"); + } + printf("two\n"); + return 0; +} + +int while_call_condition(void) { + while (returns_twice()) + f(); + return 1; +} + +int logical_and_call(void) { + if (returns_twice() && g()) + return 1; + return 2; +} + +int musttail_call(int x) { + __attribute__((musttail)) return tail_callee(x); +} + +// IR-LABEL: define{{.*}} i32 @block_after_call( +// IR: call{{.*}} @printf( +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_block_after_call +// IR: call{{.*}} @printf( +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_block_after_call + +// IR-LABEL: define{{.*}} i32 @while_call_condition( +// IR: call{{.*}} @returns_twice +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_while_call_condition + +// IR-LABEL: define{{.*}} i32 @logical_and_call( +// IR: call{{.*}} @returns_twice +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_logical_and_call +// IR: call{{.*}} @g +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_logical_and_call + +// IR-LABEL: define{{.*}} i32 @musttail_call( +// IR: musttail call i32 @tail_callee +// IR-NEXT: ret i32 +// IR-NOT: getelementptr inbounds ({{.*}}@__profc_musttail_call + +// MAP-LABEL: block_after_call: +// MAP: Gap,File 0, [[BLOCK_CLOSE:[0-9]+]]:4 -> [[BLOCK_NEXT:[0-9]+]]:3 = #2 +// MAP-NEXT: File 0, [[BLOCK_NEXT]]:3 -> [[BLOCK_NEXT]]:18 = #2 + +// MAP-LABEL: while_call_condition: +// MAP: File 0, [[WHILE_COND:[0-9]+]]:10 -> [[WHILE_COND]]:25 = (#0 + #3) +// MAP-NEXT: Branch,File 0, [[WHILE_COND]]:10 -> [[WHILE_COND]]:25 = #1, (#2 - #1) + +// MAP-LABEL: logical_and_call: +// MAP: Branch,File 0, [[LAND_COND:[0-9]+]]:7 -> [[LAND_COND]]:22 = #2, (#4 - #2) +// MAP: Branch,File 0, [[LAND_COND]]:26 -> [[LAND_COND]]:29 = #3, (#5 - #3) + +// MAP-LABEL: musttail_call: +// MAP-NEXT: File 0, {{[0-9]+}}:26 -> {{[0-9]+}}:2 = #0 >From c625d6d5631763a8b0617f7fe5d0fbd50939b6b3 Mon Sep 17 00:00:00 2001 From: "[email protected]" <[email protected]> Date: Mon, 15 Jun 2026 15:53:06 +0000 Subject: [PATCH 3/5] [clang][coverage] Move call continuation IR tests --- .../call-continuations-tight.c | 23 +---- .../test/CoverageMapping/call-continuations.c | 18 ++-- clang/test/Profile/call-continuations.c | 89 +++++++++++++++++++ 3 files changed, 95 insertions(+), 35 deletions(-) create mode 100644 clang/test/Profile/call-continuations.c diff --git a/clang/test/CoverageMapping/call-continuations-tight.c b/clang/test/CoverageMapping/call-continuations-tight.c index 11b01870cf620..de3787585c6db 100644 --- a/clang/test/CoverageMapping/call-continuations-tight.c +++ b/clang/test/CoverageMapping/call-continuations-tight.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -dump-coverage-mapping -emit-llvm -o - %s | FileCheck %s --check-prefixes=IR,MAP +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -dump-coverage-mapping -emit-llvm-only -o - %s | FileCheck %s --check-prefix=MAP int printf(const char *, ...); void f(void); @@ -32,27 +32,6 @@ int musttail_call(int x) { __attribute__((musttail)) return tail_callee(x); } -// IR-LABEL: define{{.*}} i32 @block_after_call( -// IR: call{{.*}} @printf( -// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_block_after_call -// IR: call{{.*}} @printf( -// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_block_after_call - -// IR-LABEL: define{{.*}} i32 @while_call_condition( -// IR: call{{.*}} @returns_twice -// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_while_call_condition - -// IR-LABEL: define{{.*}} i32 @logical_and_call( -// IR: call{{.*}} @returns_twice -// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_logical_and_call -// IR: call{{.*}} @g -// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_logical_and_call - -// IR-LABEL: define{{.*}} i32 @musttail_call( -// IR: musttail call i32 @tail_callee -// IR-NEXT: ret i32 -// IR-NOT: getelementptr inbounds ({{.*}}@__profc_musttail_call - // MAP-LABEL: block_after_call: // MAP: Gap,File 0, [[BLOCK_CLOSE:[0-9]+]]:4 -> [[BLOCK_NEXT:[0-9]+]]:3 = #2 // MAP-NEXT: File 0, [[BLOCK_NEXT]]:3 -> [[BLOCK_NEXT]]:18 = #2 diff --git a/clang/test/CoverageMapping/call-continuations.c b/clang/test/CoverageMapping/call-continuations.c index ef4d430267958..4792fe2622039 100644 --- a/clang/test/CoverageMapping/call-continuations.c +++ b/clang/test/CoverageMapping/call-continuations.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -dump-coverage-mapping -emit-llvm -o - %s | FileCheck %s --check-prefixes=MAP,IR +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -dump-coverage-mapping -emit-llvm-only -o - %s | FileCheck %s --check-prefix=MAP // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -o - %s | FileCheck %s --check-prefix=NOCC void f(void); @@ -9,24 +9,16 @@ int after_call(void) { return 1; } -// IR-LABEL: define{{.*}} i32 @after_call( -// IR: call void @f() -// IR-NEXT: load i64, ptr getelementptr inbounds ([2 x i64], ptr @__profc_after_call, i32 0, i32 1) -// IR: ret i32 1 - int setjmp_like(void) { if (returns_twice() == 0) return 1; return 2; } -// IR-LABEL: define{{.*}} i32 @setjmp_like -// IR: call{{.*}}@returns_twice -// IR-NEXT: load i64, ptr getelementptr inbounds ([3 x i64], ptr @__profc_setjmp_like, i32 0, i32 2) // MAP-LABEL: after_call: -// MAP: Gap,File 0, [[@LINE-19]]:7 -> [[@LINE-18]]:3 = #1 -// MAP: File 0, [[@LINE-19]]:3 -> [[@LINE-18]]:2 = #1 +// MAP: Gap,File 0, [[CALL_LINE:[0-9]+]]:7 -> [[RET_LINE:[0-9]+]]:3 = #1 +// MAP: File 0, [[RET_LINE]]:3 -> [[END_LINE:[0-9]+]]:2 = #1 // MAP-LABEL: setjmp_like: -// MAP: Branch,File 0, [[@LINE-12]]:7 -> [[@LINE-12]]:27 = #1, (#2 - #1) +// MAP: Branch,File 0, [[COND_LINE:[0-9]+]]:7 -> [[COND_LINE]]:27 = #1, (#2 - #1) // NOCC-LABEL: setjmp_like: -// NOCC: Branch,File 0, [[@LINE-14]]:7 -> [[@LINE-14]]:27 = #1, (#0 - #1) +// NOCC: Branch,File 0, [[COND_LINE:[0-9]+]]:7 -> [[COND_LINE]]:27 = #1, (#0 - #1) diff --git a/clang/test/Profile/call-continuations.c b/clang/test/Profile/call-continuations.c new file mode 100644 index 0000000000000..9e087b323d4a4 --- /dev/null +++ b/clang/test/Profile/call-continuations.c @@ -0,0 +1,89 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -emit-llvm -o - %s | FileCheck %s --check-prefix=IR +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -mllvm -enable-single-byte-coverage=true -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -emit-llvm -o - %s | FileCheck %s --check-prefix=SB + +int printf(const char *, ...); +void f(void); +int g(void); +__attribute__((returns_twice)) int returns_twice(void); +int tail_callee(int); + +int after_call(void) { + f(); + return 1; +} + +int setjmp_like(void) { + if (returns_twice() == 0) + return 1; + return 2; +} + +int block_after_call(int argc) { + { + if (argc > 2) + return 2; + printf("one\n"); + } + printf("two\n"); + return 0; +} + +int while_call_condition(void) { + while (returns_twice()) + f(); + return 1; +} + +int logical_and_call(void) { + if (returns_twice() && g()) + return 1; + return 2; +} + +int musttail_call(int x) { + __attribute__((musttail)) return tail_callee(x); +} + +// IR-LABEL: define{{.*}} i32 @after_call( +// IR: call void @f() +// IR-NEXT: load i64, ptr getelementptr inbounds ([2 x i64], ptr @__profc_after_call, i32 0, i32 1) +// IR: ret i32 1 + +// IR-LABEL: define{{.*}} i32 @setjmp_like( +// IR: call{{.*}} @returns_twice +// IR-NEXT: load i64, ptr getelementptr inbounds ([3 x i64], ptr @__profc_setjmp_like, i32 0, i32 2) + +// IR-LABEL: define{{.*}} i32 @block_after_call( +// IR: call{{.*}} @printf( +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_block_after_call +// IR: call{{.*}} @printf( +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_block_after_call + +// IR-LABEL: define{{.*}} i32 @while_call_condition( +// IR: call{{.*}} @returns_twice +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_while_call_condition + +// IR-LABEL: define{{.*}} i32 @logical_and_call( +// IR: call{{.*}} @returns_twice +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_logical_and_call +// IR: call{{.*}} @g +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_logical_and_call + +// IR-LABEL: define{{.*}} i32 @musttail_call( +// IR: musttail call i32 @tail_callee +// IR-NEXT: ret i32 +// IR-NOT: getelementptr inbounds ({{.*}}@__profc_musttail_call + +// SB-LABEL: define{{.*}} i32 @after_call( +// SB: call void @f() +// SB-NEXT: store i8 0, ptr getelementptr inbounds ([2 x i8], ptr @__profc_after_call, i32 0, i32 1) +// SB: ret i32 1 + +// SB-LABEL: define{{.*}} i32 @setjmp_like( +// SB: call{{.*}} @returns_twice +// SB-NEXT: store i8 0, ptr getelementptr inbounds ([4 x i8], ptr @__profc_setjmp_like, i32 0, i32 2) + +// SB-LABEL: define{{.*}} i32 @musttail_call( +// SB: musttail call i32 @tail_callee +// SB-NEXT: ret i32 +// SB-NOT: getelementptr inbounds ({{.*}}@__profc_musttail_call >From 4f081e697094737ae923bb51da0e9c808d51ed07 Mon Sep 17 00:00:00 2001 From: "[email protected]" <[email protected]> Date: Thu, 18 Jun 2026 14:28:35 +0000 Subject: [PATCH 4/5] [clang][coverage] Handle call continuation regions after macro calls --- clang/lib/CodeGen/CoverageMappingGen.cpp | 85 +++++++++++++++++-- .../test/CoverageMapping/call-continuations.c | 49 +++++++++++ 2 files changed, 127 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index ac70667040727..4daa29caf609c 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -356,6 +356,20 @@ class CoverageMappingBuilder { return Loc; } + /// Get the start of \c S for a region that has not started yet. + SourceLocation getStartOfNewRegion(const Stmt *S) { + SourceLocation Loc = getStart(S); + if (!Loc.isMacroID()) + return Loc; + + FileID FID = SM.getFileID(Loc); + const SrcMgr::ExpansionInfo *Expansion = + &SM.getSLocEntry(FID).getExpansion(); + if (Expansion->isFunctionMacroExpansion()) + return Expansion->getExpansionLocStart(); + return Loc; + } + /// Get the end of \c S ignoring macro arguments and builtin macros. SourceLocation getEnd(const Stmt *S) { SourceLocation Loc = S->getEndLoc(); @@ -930,6 +944,9 @@ struct CounterCoverageMappingBuilder /// A stack of currently live regions. llvm::SmallVector<SourceMappingRegion> RegionStack; + /// RegionStack indices for regions created by startCallContinuationRegion(). + llvm::SmallSet<size_t, 4> CallContinuationRegionIndices; + /// Set if the Expr should be handled as a leaf even if it is kind of binary /// logical ops (&&, ||). llvm::DenseSet<const Stmt *> LeafExprSet; @@ -1120,6 +1137,52 @@ struct CounterCoverageMappingBuilder return Depth; } + /// Return whether a region would remain in source order after popRegions() + /// adjusts it out of nested macro expansions or included files. + bool isRegionInSourceOrder(SourceLocation StartLoc, SourceLocation EndLoc) { + if (StartLoc.isInvalid() || EndLoc.isInvalid()) + return false; + + size_t StartDepth = locationDepth(StartLoc); + size_t EndDepth = locationDepth(EndLoc); + while (!SM.isWrittenInSameFile(StartLoc, EndLoc)) { + bool UnnestStart = StartDepth >= EndDepth; + bool UnnestEnd = EndDepth >= StartDepth; + if (UnnestEnd) { + EndLoc = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(EndLoc)); + if (EndLoc.isInvalid()) + return false; + EndDepth--; + } + if (UnnestStart) { + StartLoc = getIncludeOrExpansionLoc(StartLoc); + if (StartLoc.isInvalid()) + return false; + StartDepth--; + } + } + + return SpellingRegion(SM, StartLoc, EndLoc).isInSourceOrder(); + } + + /// Return the closest enclosing end location for the active region. + std::optional<SourceLocation> getRegionEndLoc(size_t ParentIndex, + SourceLocation StartLoc) { + for (size_t I = RegionStack.size() - 1; I > ParentIndex; --I) { + if (!RegionStack[I - 1].hasEndLoc()) + continue; + SourceLocation EndLoc = RegionStack[I - 1].getEndLoc(); + if (isRegionInSourceOrder(StartLoc, EndLoc)) + return EndLoc; + } + if (RegionStack[ParentIndex].hasEndLoc()) { + SourceLocation EndLoc = RegionStack[ParentIndex].getEndLoc(); + if (isRegionInSourceOrder(StartLoc, EndLoc)) + return EndLoc; + } + return std::nullopt; + } + /// Pop regions from the stack into the function's list of regions. /// /// Adds all regions from \c ParentIndex to the top of the stack to the @@ -1128,12 +1191,14 @@ struct CounterCoverageMappingBuilder assert(RegionStack.size() >= ParentIndex && "parent not in stack"); while (RegionStack.size() > ParentIndex) { SourceMappingRegion &Region = RegionStack.back(); - if (Region.hasStartLoc() && - (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) { + std::optional<SourceLocation> RegionEndLoc; + if (Region.hasEndLoc()) + RegionEndLoc = Region.getEndLoc(); + else if (Region.hasStartLoc()) + RegionEndLoc = getRegionEndLoc(ParentIndex, Region.getBeginLoc()); + if (Region.hasStartLoc() && RegionEndLoc) { SourceLocation StartLoc = Region.getBeginLoc(); - SourceLocation EndLoc = Region.hasEndLoc() - ? Region.getEndLoc() - : RegionStack[ParentIndex].getEndLoc(); + SourceLocation EndLoc = *RegionEndLoc; bool isBranch = Region.isBranch(); size_t StartDepth = locationDepth(StartLoc); size_t EndDepth = locationDepth(EndLoc); @@ -1197,6 +1262,7 @@ struct CounterCoverageMappingBuilder assert(SpellingRegion(SM, Region).isInSourceOrder()); SourceRegions.push_back(Region); } + CallContinuationRegionIndices.erase(RegionStack.size() - 1); RegionStack.pop_back(); } } @@ -1395,7 +1461,11 @@ struct CounterCoverageMappingBuilder /// Ensure that \c S is included in the current region. void extendRegion(const Stmt *S) { SourceMappingRegion &Region = getRegion(); - SourceLocation StartLoc = getStart(S); + bool IsStartingCallContinuation = + !Region.hasStartLoc() && + CallContinuationRegionIndices.count(RegionStack.size() - 1); + SourceLocation StartLoc = + IsStartingCallContinuation ? getStartOfNewRegion(S) : getStart(S); handleFileExit(StartLoc); if (!Region.hasStartLoc()) @@ -1423,7 +1493,8 @@ struct CounterCoverageMappingBuilder Region.hasEndLoc() ? std::optional<SourceLocation>(Region.getEndLoc()) : std::nullopt; Region.setEndLoc(EndLoc); - pushRegion(ContinuationCount, std::nullopt, OriginalEndLoc); + size_t Index = pushRegion(ContinuationCount, std::nullopt, OriginalEndLoc); + CallContinuationRegionIndices.insert(Index); GapRegionCounter = ContinuationCount; HasGapRegion = true; } diff --git a/clang/test/CoverageMapping/call-continuations.c b/clang/test/CoverageMapping/call-continuations.c index 4792fe2622039..4a8f0e972ff32 100644 --- a/clang/test/CoverageMapping/call-continuations.c +++ b/clang/test/CoverageMapping/call-continuations.c @@ -2,8 +2,27 @@ // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -o - %s | FileCheck %s --check-prefix=NOCC void f(void); +void *alloc(unsigned long); +void release(void *); __attribute__((returns_twice)) int returns_twice(void); +struct cleanup_ctx { + int fd0; + int fd1; +}; + +#define CLEANUP_CTX_MACRO(ctx, close0, close1) \ + { \ + if (close0) \ + f(); \ + if (close1) \ + f(); \ + (ctx)->fd0 = -1; \ + (ctx)->fd1 = -1; \ + release((void *)(ctx)); \ + ctx = (struct cleanup_ctx *)0; \ + } + int after_call(void) { f(); return 1; @@ -15,10 +34,40 @@ int setjmp_like(void) { return 2; } +int after_guard_and_call(int ret) { + void *child = alloc(4); + if (!child) + return 2; + if (ret) { + f(); + return 1; + } + return 0; +} + +int cleanup_macro_after_call(int ret) { + struct cleanup_ctx *child = (struct cleanup_ctx *)alloc(sizeof(*child)); + struct cleanup_ctx *parent = (struct cleanup_ctx *)alloc(sizeof(*parent)); + if (!child || !parent) + return 2; + if (ret) { + f(); + CLEANUP_CTX_MACRO(child, 1, 1); + CLEANUP_CTX_MACRO(parent, 1, 1); + return 1; + } + return 0; +} + // MAP-LABEL: after_call: // MAP: Gap,File 0, [[CALL_LINE:[0-9]+]]:7 -> [[RET_LINE:[0-9]+]]:3 = #1 // MAP: File 0, [[RET_LINE]]:3 -> [[END_LINE:[0-9]+]]:2 = #1 // MAP-LABEL: setjmp_like: // MAP: Branch,File 0, [[COND_LINE:[0-9]+]]:7 -> [[COND_LINE]]:27 = #1, (#2 - #1) +// MAP-LABEL: after_guard_and_call: +// MAP: Gap,File 0, [[CALL_LINE:[0-9]+]]:9 -> [[RET_LINE:[0-9]+]]:5 = #{{[0-9]+}} +// MAP-LABEL: cleanup_macro_after_call: +// MAP: Expansion,File 0, [[FIRST_CLEANUP:[0-9]+]]:5 -> [[FIRST_CLEANUP]]:22 = #{{[0-9]+}} +// MAP: Expansion,File 0, [[SECOND_CLEANUP:[0-9]+]]:5 -> [[SECOND_CLEANUP]]:22 = #{{[0-9]+}} // NOCC-LABEL: setjmp_like: // NOCC: Branch,File 0, [[COND_LINE:[0-9]+]]:7 -> [[COND_LINE]]:27 = #1, (#0 - #1) >From bedb7ffcce0bb08cfef75cb0cad3aedf30c0ecfd Mon Sep 17 00:00:00 2001 From: "[email protected]" <[email protected]> Date: Fri, 26 Jun 2026 03:40:16 +0000 Subject: [PATCH 5/5] [clang][coverage] Tighten call continuation edge cases While testing the continuation counters more closely, I found a few places where the mapping could reserve or use the wrong count. The main fixes are: skip unevaluated calls such as sizeof(f()), feed a for-loop condition from the increment's exit count, handle C++ constructor calls and written constructor initializers, and keep coverage-expression minimizer state separate from remapped ids. That last bit matters because expression ids 0 and 1 are real output ids, so they cannot also be used as internal marker values. --- clang/lib/CodeGen/CGExprCXX.cpp | 2 + clang/lib/CodeGen/CodeGenPGO.cpp | 104 ++++++++++++++++-- clang/lib/CodeGen/CoverageMappingGen.cpp | 25 ++++- .../call-continuations-cxx.cpp | 23 ++++ .../call-continuations-tight.c | 19 ++++ clang/test/Profile/call-continuations-cxx.cpp | 26 +++++ clang/test/Profile/call-continuations-hash.c | 12 ++ clang/test/Profile/call-continuations.c | 22 ++++ .../Coverage/CoverageMappingWriter.cpp | 30 +++-- .../ProfileData/CoverageMappingTest.cpp | 28 +++++ 10 files changed, 267 insertions(+), 24 deletions(-) create mode 100644 clang/test/CoverageMapping/call-continuations-cxx.cpp create mode 100644 clang/test/Profile/call-continuations-cxx.cpp create mode 100644 clang/test/Profile/call-continuations-hash.c diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index ebbc0addfed2c..fbc3ddc6dd99c 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -679,6 +679,8 @@ void CodeGenFunction::EmitCXXConstructExpr(const CXXConstructExpr *E, // Call the constructor. EmitCXXConstructorCall(CD, Type, ForVirtualBase, Delegating, Dest, E); } + + incrementCallContinuationProfileCounter(E); } void CodeGenFunction::EmitSynthesizedCXXCopyCtor(Address Dest, Address Src, diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index d5e41e2d13196..eaf18ca98d727 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -15,6 +15,7 @@ #include "CodeGenFunction.h" #include "CoverageMappingGen.h" #include "clang/AST/Attr.h" +#include "clang/AST/EvaluatedExprVisitor.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/StmtVisitor.h" #include "clang/Basic/DiagnosticFrontend.h" @@ -135,6 +136,8 @@ class PGOHash { BinaryOperatorNE, // The preceding values are available since PGO_HASH_V2. + CallContinuationCounters, + // Keep this last. It's for the static assert that follows. LastHashType }; @@ -170,12 +173,14 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { unsigned NextCounter; /// The function hash. PGOHash Hash; + ASTContext &Context; /// The map of statements to counters. llvm::DenseMap<const Stmt *, CounterPair> &CounterMap; /// The map of calls to counters reached only when the call returns. llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap; - /// Calls that need continuation counters assigned after normal counters. - SmallVector<const CallExpr *, 8> CallContinuations; + /// Call-like expressions that need continuation counters assigned after + /// normal counters. + SmallVector<const Stmt *, 8> CallContinuations; /// The state of MC/DC Coverage in this function. MCDC::State &MCDCState; /// Maximum number of supported MC/DC conditions in a boolean expression. @@ -194,8 +199,10 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { llvm::DenseMap<const Stmt *, CounterPair> &CounterMap, llvm::DenseMap<const Stmt *, unsigned> *CallContinuationCounterMap, MCDC::State &MCDCState, unsigned MCDCMaxCond, - bool CoverageCallContinuations, DiagnosticsEngine &Diag) - : NextCounter(0), Hash(HashVersion), CounterMap(CounterMap), + bool CoverageCallContinuations, ASTContext &Context, + DiagnosticsEngine &Diag) + : NextCounter(0), Hash(HashVersion), Context(Context), + CounterMap(CounterMap), CallContinuationCounterMap(CallContinuationCounterMap), MCDCState(MCDCState), MCDCMaxCond(MCDCMaxCond), CoverageCallContinuations(CoverageCallContinuations), @@ -386,21 +393,91 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { return getFunctionExtInfo(*CalleeType).getNoReturn(); } - /// Calls optionally get a continuation counter which is reached only if the - /// call returns normally. This lets coverage mapping distinguish a call being - /// reached from later source being reached. - bool VisitCallExpr(CallExpr *S) { + bool shouldEmitCXXConstructContinuation(const CXXConstructExpr *E) const { + const CXXConstructorDecl *CD = E->getConstructor(); + if (CD->isNoReturn()) + return false; + if (CD->isTrivial() && CD->isDefaultConstructor()) + return false; + if (Context.getLangOpts().ElideConstructors && E->isElidable()) + return false; + return true; + } + + void addCallContinuation(const CallExpr *S, const CallExpr *CurrentMustTail) { + if (CoverageCallContinuations && CallContinuationCounterMap && + !isNoReturnCall(S) && S != CurrentMustTail) + CallContinuations.push_back(S); + } + + void addCallContinuation(const CXXConstructExpr *S) { if (CoverageCallContinuations && CallContinuationCounterMap && - !isNoReturnCall(S) && S != MustTailCall) + shouldEmitCXXConstructContinuation(S)) CallContinuations.push_back(S); - return Base::VisitCallExpr(S); + } + + // Continuation counters are emitted from codegen, so collection has to match + // codegen's notion of evaluated calls. The normal counter walk sees the full + // AST; this pass avoids reserving counters for calls in sizeof(f())-style + // unevaluated contexts that will never be emitted. + struct CollectCallContinuations + : public ConstEvaluatedExprVisitor<CollectCallContinuations> { + MapRegionCounters &Owner; + const CallExpr *MustTailCall = nullptr; + + CollectCallContinuations(ASTContext &Context, MapRegionCounters &Owner) + : ConstEvaluatedExprVisitor(Context), Owner(Owner) {} + + void VisitAttributedStmt(const AttributedStmt *S) { + const CallExpr *NewMustTailCall = getMustTailCall(S); + if (!NewMustTailCall) + return VisitStmt(S); + + llvm::SaveAndRestore<const CallExpr *> SaveMustTail(MustTailCall, + NewMustTailCall); + Visit(S->getSubStmt()); + } + + void VisitCallExpr(const CallExpr *S) { + if (S->isUnevaluatedBuiltinCall(this->Context)) + return; + + Owner.addCallContinuation(S, MustTailCall); + VisitExpr(S); + } + + void VisitCXXConstructExpr(const CXXConstructExpr *S) { + Owner.addCallContinuation(S); + VisitStmt(S); + } + }; + + void collectCallContinuations(const Decl *D) { + if (!CoverageCallContinuations || !CallContinuationCounterMap) + return; + + CollectCallContinuations Collector(Context, *this); + if (const auto *Ctor = dyn_cast_or_null<CXXConstructorDecl>(D)) { + for (const CXXCtorInitializer *Initializer : Ctor->inits()) { + if (Initializer->isWritten()) + Collector.Visit(Initializer->getInit()); + } + Collector.Visit(Ctor->getBody()); + } else if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D)) + Collector.Visit(FD->getBody()); + else if (const auto *MD = dyn_cast_or_null<ObjCMethodDecl>(D)) + Collector.Visit(MD->getBody()); + else if (const auto *BD = dyn_cast_or_null<BlockDecl>(D)) + Collector.Visit(BD->getBody()); + else if (const auto *CD = dyn_cast_or_null<CapturedDecl>(D)) + Collector.Visit(CD->getBody()); } void assignCallContinuationCounters() { if (!CallContinuationCounterMap) return; - for (const CallExpr *S : CallContinuations) + for (const Stmt *S : CallContinuations) (*CallContinuationCounterMap)[S] = NextCounter++; } @@ -1067,7 +1144,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) { MapRegionCounters Walker(HashVersion, ProfileVersion, *RegionCounterMap, CallContinuationCounterMap.get(), *RegionMCDCState, MCDCMaxConditions, CoverageCallContinuations, - CGM.getDiags()); + CGM.getContext(), CGM.getDiags()); if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) Walker.TraverseDecl(const_cast<FunctionDecl *>(FD)); else if (const ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(D)) @@ -1076,9 +1153,12 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) { Walker.TraverseDecl(const_cast<BlockDecl *>(BD)); else if (const CapturedDecl *CD = dyn_cast_or_null<CapturedDecl>(D)) Walker.TraverseDecl(const_cast<CapturedDecl *>(CD)); + Walker.collectCallContinuations(D); Walker.assignCallContinuationCounters(); assert(Walker.NextCounter > 0 && "no entry counter mapped for decl"); NumRegionCounters = Walker.NextCounter; + if (CoverageCallContinuations) + Walker.Hash.combine(PGOHash::CallContinuationCounters); FunctionHash = Walker.Hash.finalize(); if (HashVersion >= PGO_HASH_V4) FunctionHash &= llvm::NamedInstrProfRecord::FUNC_HASH_MASK; diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 4daa29caf609c..1df064b7b8f80 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -1717,8 +1717,11 @@ struct CounterCoverageMappingBuilder for (auto *Initializer : Ctor->inits()) { if (Initializer->isWritten()) { auto *Init = Initializer->getInit(); + // Written initializers run before the constructor body. Seed the + // body with the initializer's exit count so a non-returning call in + // the initializer does not make the body look covered. if (getStart(Init).isValid() && getEnd(Init).isValid()) - propagateCounts(BodyCounter, Init); + BodyCounter = propagateCounts(BodyCounter, Init); } } } @@ -1798,6 +1801,13 @@ struct CounterCoverageMappingBuilder startCallContinuationRegion(E, *ContinuationCounter); } + void VisitCXXConstructExpr(const CXXConstructExpr *E) { + VisitStmt(E); + if (std::optional<Counter> ContinuationCounter = + getCallContinuationCounter(E)) + startCallContinuationRegion(E, *ContinuationCounter); + } + void VisitWhileStmt(const WhileStmt *S) { extendRegion(S); @@ -1902,15 +1912,20 @@ struct CounterCoverageMappingBuilder // The increment is essentially part of the body but it needs to include // the count for all the continue statements. BreakContinue IncrementBC; + Counter IncrementCount = addCounters(BackedgeCount, BodyBC.ContinueCount); + Counter IncrementExitCount = IncrementCount; if (const Stmt *Inc = S->getInc()) { - propagateCounts(addCounters(BackedgeCount, BodyBC.ContinueCount), Inc); + // for (...; ...; f()) only reaches the next condition evaluation if the + // increment returns normally. Use the propagated exit count here instead + // of feeding the pre-increment count straight back to the condition. + IncrementExitCount = propagateCounts(IncrementCount, Inc); IncrementBC = BreakContinueStack.pop_back_val(); } // Go back to handle the condition. - Counter CondCount = addCounters( - addCounters(ParentCount, BackedgeCount, BodyBC.ContinueCount), - IncrementBC.ContinueCount); + Counter CondCount = + addCounters(addCounters(ParentCount, IncrementExitCount), + IncrementBC.ContinueCount); Counter CondExitCount = CondCount; if (const Expr *Cond = S->getCond()) { diff --git a/clang/test/CoverageMapping/call-continuations-cxx.cpp b/clang/test/CoverageMapping/call-continuations-cxx.cpp new file mode 100644 index 0000000000000..7bc42629ada7d --- /dev/null +++ b/clang/test/CoverageMapping/call-continuations-cxx.cpp @@ -0,0 +1,23 @@ +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -dump-coverage-mapping -emit-llvm-only -o - %s | FileCheck %s --check-prefix=MAP + +int init_value(); + +struct C { + int x; + C() : x(init_value()) { + x = 2; + } +}; + +int constructor_call(void) { + C c; + return c.x; +} + +// MAP-LABEL: _Z16constructor_callv: +// MAP: Gap,File 0, [[CTOR_LINE:[0-9]+]]:7 -> [[RET_LINE:[0-9]+]]:3 = #1 +// MAP-NEXT: File 0, [[RET_LINE]]:3 -> {{[0-9]+}}:2 = #1 + +// MAP-LABEL: _ZN1CC2Ev: +// MAP: File 0, [[INIT_LINE:[0-9]+]]:11 -> [[INIT_LINE]]:23 = #0 +// MAP-NEXT: File 0, [[INIT_LINE]]:25 -> {{[0-9]+}}:4 = #1 diff --git a/clang/test/CoverageMapping/call-continuations-tight.c b/clang/test/CoverageMapping/call-continuations-tight.c index de3787585c6db..4d0b19a2bbd68 100644 --- a/clang/test/CoverageMapping/call-continuations-tight.c +++ b/clang/test/CoverageMapping/call-continuations-tight.c @@ -28,6 +28,17 @@ int logical_and_call(void) { return 2; } +int unevaluated_sizeof(void) { + int x = sizeof(g()); + return x == sizeof(int) ? 0 : 1; +} + +int for_increment_call(void) { + for (int i = 0; i < g(); f()) + g(); + return 0; +} + int musttail_call(int x) { __attribute__((musttail)) return tail_callee(x); } @@ -44,5 +55,13 @@ int musttail_call(int x) { // MAP: Branch,File 0, [[LAND_COND:[0-9]+]]:7 -> [[LAND_COND]]:22 = #2, (#4 - #2) // MAP: Branch,File 0, [[LAND_COND]]:26 -> [[LAND_COND]]:29 = #3, (#5 - #3) +// MAP-LABEL: unevaluated_sizeof: +// MAP: File 0, [[SIZEOF_START:[0-9]+]]:30 -> [[SIZEOF_END:[0-9]+]]:2 = #0 +// MAP: Branch,File 0, [[SIZEOF_RET:[0-9]+]]:10 -> [[SIZEOF_RET]]:26 = #1, (#0 - #1) + +// MAP-LABEL: for_increment_call: +// MAP: File 0, [[FOR_COND:[0-9]+]]:19 -> [[FOR_COND]]:26 = (#0 + #{{[0-9]+}}) +// MAP: Branch,File 0, [[FOR_COND]]:19 -> [[FOR_COND]]:26 = #{{[0-9]+}}, (#{{[0-9]+}} - #{{[0-9]+}}) + // MAP-LABEL: musttail_call: // MAP-NEXT: File 0, {{[0-9]+}}:26 -> {{[0-9]+}}:2 = #0 diff --git a/clang/test/Profile/call-continuations-cxx.cpp b/clang/test/Profile/call-continuations-cxx.cpp new file mode 100644 index 0000000000000..b797625bfb00f --- /dev/null +++ b/clang/test/Profile/call-continuations-cxx.cpp @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -emit-llvm -o - %s | FileCheck %s --check-prefix=IR + +int init_value(); + +struct C { + int x; + C() : x(init_value()) { + x = 2; + } +}; + +int constructor_call(void) { + C c; + return c.x; +} + +// IR-DAG: @__profc__Z16constructor_callv = private global [2 x i64] +// IR-DAG: @__profc__ZN1CC2Ev = linkonce_odr hidden global [2 x i64] + +// IR-LABEL: define{{.*}} i32 @_Z16constructor_callv( +// IR: call void @_ZN1CC1Ev +// IR-NEXT: load i64, ptr getelementptr inbounds ([2 x i64], ptr @__profc__Z16constructor_callv, i32 0, i32 1) + +// IR-LABEL: define{{.*}} void @_ZN1CC2Ev( +// IR: call{{.*}} @_Z10init_valuev +// IR-NEXT: load i64, ptr getelementptr inbounds ([2 x i64], ptr @__profc__ZN1CC2Ev, i32 0, i32 1) diff --git a/clang/test/Profile/call-continuations-hash.c b/clang/test/Profile/call-continuations-hash.c new file mode 100644 index 0000000000000..ce6f1a6e6483d --- /dev/null +++ b/clang/test/Profile/call-continuations-hash.c @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -main-file-name call-continuations-hash.c -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -o - %s | FileCheck %s --check-prefix=NOCC +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -main-file-name call-continuations-hash.c -fprofile-instrument=clang -fcoverage-mapping -fcoverage-call-continuations -emit-llvm -o - %s | FileCheck %s --check-prefix=CC + +void f(void); + +int foo(void) { + f(); + return 1; +} + +// NOCC: @__profd_foo = private global {{.*}} { i64 {{[-0-9]+}}, i64 24, {{.*}}, i32 1, +// CC: @__profd_foo = private global {{.*}} { i64 {{[-0-9]+}}, i64 1569, {{.*}}, i32 2, diff --git a/clang/test/Profile/call-continuations.c b/clang/test/Profile/call-continuations.c index 9e087b323d4a4..9bcbe9ba00a65 100644 --- a/clang/test/Profile/call-continuations.c +++ b/clang/test/Profile/call-continuations.c @@ -40,10 +40,24 @@ int logical_and_call(void) { return 2; } +int unevaluated_sizeof(void) { + int x = sizeof(g()); + return x == sizeof(int) ? 0 : 1; +} + +int for_increment_call(void) { + for (int i = 0; i < g(); f()) + g(); + return 0; +} + int musttail_call(int x) { __attribute__((musttail)) return tail_callee(x); } +// IR-DAG: @__profc_unevaluated_sizeof = private global [2 x i64] +// SB-DAG: @__profc_unevaluated_sizeof = private global [3 x i8] + // IR-LABEL: define{{.*}} i32 @after_call( // IR: call void @f() // IR-NEXT: load i64, ptr getelementptr inbounds ([2 x i64], ptr @__profc_after_call, i32 0, i32 1) @@ -69,6 +83,14 @@ int musttail_call(int x) { // IR: call{{.*}} @g // IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_logical_and_call +// IR-LABEL: define{{.*}} i32 @unevaluated_sizeof( +// IR-NOT: call{{.*}} @g +// IR: ret i32 + +// IR-LABEL: define{{.*}} i32 @for_increment_call( +// IR: call{{.*}} @f +// IR-NEXT: load i64, ptr getelementptr inbounds ({{.*}}@__profc_for_increment_call + // IR-LABEL: define{{.*}} i32 @musttail_call( // IR: musttail call i32 @tail_callee // IR-NEXT: ret i32 diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp index dc97870985145..470724f77b2e3 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp @@ -73,11 +73,17 @@ class CounterExpressionsMinimizer { SmallVector<CounterExpression, 16> UsedExpressions; std::vector<unsigned> AdjustedExpressionIDs; + // Expression id 0 is a valid remapped id, so keep the traversal state + // separate from assigned ids. Otherwise a shared expression can be gathered + // twice and make later expression ids too large to encode. + static constexpr unsigned Unused = std::numeric_limits<unsigned>::max(); + static constexpr unsigned Marked = Unused - 1; + public: CounterExpressionsMinimizer(ArrayRef<CounterExpression> Expressions, ArrayRef<CounterMappingRegion> MappingRegions) : Expressions(Expressions) { - AdjustedExpressionIDs.resize(Expressions.size(), 0); + AdjustedExpressionIDs.resize(Expressions.size(), Unused); for (const auto &I : MappingRegions) { mark(I.Count); mark(I.FalseCount); @@ -92,16 +98,21 @@ class CounterExpressionsMinimizer { if (!C.isExpression()) return; unsigned ID = C.getExpressionID(); - AdjustedExpressionIDs[ID] = 1; + if (AdjustedExpressionIDs[ID] != Unused) + return; + AdjustedExpressionIDs[ID] = Marked; mark(Expressions[ID].LHS); mark(Expressions[ID].RHS); } void gatherUsed(Counter C) { - if (!C.isExpression() || !AdjustedExpressionIDs[C.getExpressionID()]) + if (!C.isExpression()) return; - AdjustedExpressionIDs[C.getExpressionID()] = UsedExpressions.size(); - const auto &E = Expressions[C.getExpressionID()]; + unsigned ID = C.getExpressionID(); + if (AdjustedExpressionIDs[ID] != Marked) + return; + AdjustedExpressionIDs[ID] = UsedExpressions.size(); + const auto &E = Expressions[ID]; UsedExpressions.push_back(E); gatherUsed(E.LHS); gatherUsed(E.RHS); @@ -112,8 +123,13 @@ class CounterExpressionsMinimizer { /// Adjust the given counter to correctly transition from the old /// expression ids to the new expression ids. Counter adjust(Counter C) const { - if (C.isExpression()) - C = Counter::getExpression(AdjustedExpressionIDs[C.getExpressionID()]); + if (C.isExpression()) { + unsigned ID = C.getExpressionID(); + assert(AdjustedExpressionIDs[ID] != Unused && + AdjustedExpressionIDs[ID] != Marked && + "expression id was not gathered"); + C = Counter::getExpression(AdjustedExpressionIDs[ID]); + } return C; } }; diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp index b268aa7cdd057..fe50c961c85c1 100644 --- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp +++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp @@ -332,6 +332,34 @@ TEST(CoverageMappingTest, expression_subst) { ASSERT_TRUE(Builder.subst(Builder.subtract(LHS, RHS), MapToExpand).isZero()); } +TEST_P(CoverageMappingTest, shared_counter_expression_write_read) { + startFunction("func", 0x1234); + addExpression(CounterExpression( + CounterExpression::Add, Counter::getCounter(0), Counter::getCounter(1))); + addExpression(CounterExpression(CounterExpression::Add, + Counter::getExpression(0), + Counter::getCounter(2))); + addExpression(CounterExpression(CounterExpression::Subtract, + Counter::getExpression(0), + Counter::getCounter(3))); + addCMR(Counter::getExpression(1), "foo", 1, 1, 1, 2); + addCMR(Counter::getExpression(2), "foo", 2, 1, 2, 2); + + OutputFunctionCoverageData Output; + readCoverageRegions(writeCoverageRegions(InputFunctions.back()), Output); + + ASSERT_EQ(3u, Output.Expressions.size()); + EXPECT_EQ(CounterExpression::Add, Output.Expressions[0].Kind); + EXPECT_EQ(Counter::getExpression(1), Output.Expressions[0].LHS); + EXPECT_EQ(Counter::getCounter(2), Output.Expressions[0].RHS); + EXPECT_EQ(CounterExpression::Add, Output.Expressions[1].Kind); + EXPECT_EQ(Counter::getCounter(0), Output.Expressions[1].LHS); + EXPECT_EQ(Counter::getCounter(1), Output.Expressions[1].RHS); + EXPECT_EQ(CounterExpression::Subtract, Output.Expressions[2].Kind); + EXPECT_EQ(Counter::getExpression(1), Output.Expressions[2].LHS); + EXPECT_EQ(Counter::getCounter(3), Output.Expressions[2].RHS); +} + TEST_P(CoverageMappingTest, basic_write_read) { startFunction("func", 0x1234); addCMR(Counter::getCounter(0), "foo", 1, 1, 1, 1); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
