Author: NAKAMURA Takumi Date: 2025-01-09T17:11:07+09:00 New Revision: 397ac44f623f891d8f05d6673a95984ac0a26671
URL: https://github.com/llvm/llvm-project/commit/397ac44f623f891d8f05d6673a95984ac0a26671 DIFF: https://github.com/llvm/llvm-project/commit/397ac44f623f891d8f05d6673a95984ac0a26671.diff LOG: [Coverage] Introduce the type `CounterPair` for RegionCounterMap. NFC. (#112724) `CounterPair` can hold `<uint32_t, uint32_t>` instead of current `unsigned`, to hold also the counter number of SkipPath. For now, this change provides the skeleton and only `CounterPair::Executed` is used. Each counter number can have `None` to suppress emitting counter increment. 2nd element `Skipped` is initialized as `None` by default, since most `Stmt*` don't have a pair of counters. This change also provides stubs for the verifier. I'll provide the impl of verifier for `+Asserts` later. `markStmtAsUsed(bool, Stmt*)` may be used to inform that other side counter may not emitted. `markStmtMaybeUsed(S)` may be used for the `Stmt` and its inner will be excluded for emission in the case of skipping by constant folding. I put it into places where I found. `verifyCounterMap()` will check the coverage map and the counter map, and can be used to report inconsistency. These verifier methods shall be eliminated in `-Asserts`. https://discourse.llvm.org/t/rfc-integrating-singlebytecoverage-with-branch-coverage/82492 Added: Modified: clang/lib/CodeGen/CGDecl.cpp clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/CodeGen/CGStmt.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenFunction.h clang/lib/CodeGen/CodeGenModule.h clang/lib/CodeGen/CodeGenPGO.cpp clang/lib/CodeGen/CodeGenPGO.h clang/lib/CodeGen/CoverageMappingGen.cpp clang/lib/CodeGen/CoverageMappingGen.h Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 47b21bc9f63f04..6f3ff050cb6978 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -361,6 +361,8 @@ CodeGenFunction::AddInitializerToStaticVarDecl(const VarDecl &D, return GV; } + PGO.markStmtMaybeUsed(D.getInit()); // FIXME: Too lazy + #ifndef NDEBUG CharUnits VarSize = CGM.getContext().getTypeSizeInChars(D.getType()) + D.getFlexibleArrayInitChars(getContext()); @@ -1868,7 +1870,10 @@ void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) { // If we are at an unreachable point, we don't need to emit the initializer // unless it contains a label. if (!HaveInsertPoint()) { - if (!Init || !ContainsLabel(Init)) return; + if (!Init || !ContainsLabel(Init)) { + PGO.markStmtMaybeUsed(Init); + return; + } EnsureInsertPoint(); } @@ -1979,6 +1984,8 @@ void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) { return EmitExprAsInit(Init, &D, lv, capturedByInit); } + PGO.markStmtMaybeUsed(Init); + if (!emission.IsConstantAggregate) { // For simple scalar/complex initialization, store the value directly. LValue lv = MakeAddrLValue(Loc, type); diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index ba1cba291553b0..1bad7a722da07a 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5148,6 +5148,7 @@ std::optional<LValue> HandleConditionalOperatorLValueSimpleCase( // If the true case is live, we need to track its region. if (CondExprBool) CGF.incrementProfileCounter(E); + CGF.markStmtMaybeUsed(Dead); // If a throw expression we emit it and return an undefined lvalue // because it can't be used. if (auto *ThrowExpr = dyn_cast<CXXThrowExpr>(Live->IgnoreParens())) { diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index b282d4e0b32f05..0f27bd00422dce 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -5003,7 +5003,8 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { CGF.incrementProfileCounter(E->getRHS()); CGF.EmitBranch(FBlock); CGF.EmitBlock(FBlock); - } + } else + CGF.markStmtMaybeUsed(E->getRHS()); CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. @@ -5015,8 +5016,10 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { } // 0 && RHS: If it is safe, just elide the RHS, and return 0/false. - if (!CGF.ContainsLabel(E->getRHS())) + if (!CGF.ContainsLabel(E->getRHS())) { + CGF.markStmtMaybeUsed(E->getRHS()); return llvm::Constant::getNullValue(ResTy); + } } // If the top of the logical operator nest, reset the MCDC temp to 0. @@ -5143,7 +5146,8 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { CGF.incrementProfileCounter(E->getRHS()); CGF.EmitBranch(FBlock); CGF.EmitBlock(FBlock); - } + } else + CGF.markStmtMaybeUsed(E->getRHS()); CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. @@ -5155,8 +5159,10 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { } // 1 || RHS: If it is safe, just elide the RHS, and return 1/true. - if (!CGF.ContainsLabel(E->getRHS())) + if (!CGF.ContainsLabel(E->getRHS())) { + CGF.markStmtMaybeUsed(E->getRHS()); return llvm::ConstantInt::get(ResTy, 1); + } } // If the top of the logical operator nest, reset the MCDC temp to 0. @@ -5280,6 +5286,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { CGF.incrementProfileCounter(E); } Value *Result = Visit(live); + CGF.markStmtMaybeUsed(dead); // If the live part is a throw expression, it acts like it has a void // type, so evaluating it returns a null Value*. However, a conditional diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index a87c50b8a1cbbf..e9a8500cc19933 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -76,6 +76,7 @@ void CodeGenFunction::EmitStmt(const Stmt *S, ArrayRef<const Attr *> Attrs) { // Verify that any decl statements were handled as simple, they may be in // scope of subsequent reachable statements. assert(!isa<DeclStmt>(*S) && "Unexpected DeclStmt!"); + PGO.markStmtMaybeUsed(S); return; } @@ -875,6 +876,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) { RunCleanupsScope ExecutedScope(*this); EmitStmt(Executed); } + PGO.markStmtMaybeUsed(Skipped); return; } } @@ -2197,6 +2199,7 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) { for (unsigned i = 0, e = CaseStmts.size(); i != e; ++i) EmitStmt(CaseStmts[i]); incrementProfileCounter(&S); + PGO.markStmtMaybeUsed(S.getBody()); // Now we want to restore the saved switch instance so that nested // switches continue to function properly diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index af58fa64f86585..27ec68bd2a872d 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1616,6 +1616,8 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, // Emit the standard function epilogue. FinishFunction(BodyRange.getEnd()); + PGO.verifyCounterMap(); + // If we haven't marked the function nothrow through other means, do // a quick pass now to see if we can. if (!CurFn->doesNotThrow()) @@ -1738,6 +1740,7 @@ bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond, if (!AllowLabels && CodeGenFunction::ContainsLabel(Cond)) return false; // Contains a label. + PGO.markStmtMaybeUsed(Cond); ResultInt = Int; return true; } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index f2240f8308ce38..e2dc0b1e381684 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1620,6 +1620,13 @@ class CodeGenFunction : public CodeGenTypeCache { uint64_t LoopCount) const; public: + auto getIsCounterPair(const Stmt *S) const { return PGO.getIsCounterPair(S); } + + void markStmtAsUsed(bool Skipped, const Stmt *S) { + PGO.markStmtAsUsed(Skipped, S); + } + void markStmtMaybeUsed(const Stmt *S) { PGO.markStmtMaybeUsed(S); } + /// Increment the profiler's counter for the given statement by \p StepV. /// If \p StepV is null, the default increment is 1. void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) { diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 741b0f17da6584..d5ef1a710eb403 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -102,6 +102,50 @@ enum ForDefinition_t : bool { ForDefinition = true }; +/// The Counter with an optional additional Counter for +/// branches. `Skipped` counter can be calculated with `Executed` and +/// a common Counter (like `Parent`) as `(Parent-Executed)`. +/// +/// In SingleByte mode, Counters are binary. Subtraction is not +/// applicable (but addition is capable). In this case, both +/// `Executed` and `Skipped` counters are required. `Skipped` is +/// `None` by default. It is allocated in the coverage mapping. +/// +/// There might be cases that `Parent` could be induced with +/// `(Executed+Skipped)`. This is not always applicable. +class CounterPair { +public: + /// Optional value. + class ValueOpt { + private: + static constexpr uint32_t None = (1u << 31); /// None is allocated. + static constexpr uint32_t Mask = None - 1; + + uint32_t Val; + + public: + ValueOpt() : Val(None) {} + + ValueOpt(unsigned InitVal) { + assert(!(InitVal & ~Mask)); + Val = InitVal; + } + + bool hasValue() const { return !(Val & None); } + + operator uint32_t() const { return Val; } + }; + + ValueOpt Executed; + ValueOpt Skipped; /// May be None. + + /// Initialized with Skipped=None. + CounterPair(unsigned Val) : Executed(Val) {} + + // FIXME: Should work with {None, None} + CounterPair() : Executed(0) {} +}; + struct OrderGlobalInitsOrStermFinalizers { unsigned int priority; unsigned int lex_order; diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 17d7902f0cfbc7..792373839107f0 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -163,7 +163,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { /// The function hash. PGOHash Hash; /// The map of statements to counters. - llvm::DenseMap<const Stmt *, unsigned> &CounterMap; + llvm::DenseMap<const Stmt *, CounterPair> &CounterMap; /// The state of MC/DC Coverage in this function. MCDC::State &MCDCState; /// Maximum number of supported MC/DC conditions in a boolean expression. @@ -174,7 +174,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { DiagnosticsEngine &Diag; MapRegionCounters(PGOHashVersion HashVersion, uint64_t ProfileVersion, - llvm::DenseMap<const Stmt *, unsigned> &CounterMap, + llvm::DenseMap<const Stmt *, CounterPair> &CounterMap, MCDC::State &MCDCState, unsigned MCDCMaxCond, DiagnosticsEngine &Diag) : NextCounter(0), Hash(HashVersion), CounterMap(CounterMap), @@ -1083,7 +1083,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) { (CGM.getCodeGenOpts().MCDCCoverage ? CGM.getCodeGenOpts().MCDCMaxConds : 0); - RegionCounterMap.reset(new llvm::DenseMap<const Stmt *, unsigned>); + RegionCounterMap.reset(new llvm::DenseMap<const Stmt *, CounterPair>); RegionMCDCState.reset(new MCDC::State); MapRegionCounters Walker(HashVersion, ProfileVersion, *RegionCounterMap, *RegionMCDCState, MCDCMaxConditions, CGM.getDiags()); @@ -1185,12 +1185,23 @@ CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader *PGOReader, Fn->setEntryCount(FunctionCount); } +std::pair<bool, bool> CodeGenPGO::getIsCounterPair(const Stmt *S) const { + if (!RegionCounterMap) + return {false, false}; + + auto I = RegionCounterMap->find(S); + if (I == RegionCounterMap->end()) + return {false, false}; + + return {I->second.Executed.hasValue(), I->second.Skipped.hasValue()}; +} + void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, llvm::Value *StepV) { if (!RegionCounterMap || !Builder.GetInsertBlock()) return; - unsigned Counter = (*RegionCounterMap)[S]; + unsigned Counter = (*RegionCounterMap)[S].Executed; // Make sure that pointer to global is passed in with zero addrspace // This is relevant during GPU profiling diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index 9d66ffad6f4350..1944b640951d5c 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -35,7 +35,7 @@ class CodeGenPGO { std::array <unsigned, llvm::IPVK_Last + 1> NumValueSites; unsigned NumRegionCounters; uint64_t FunctionHash; - std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionCounterMap; + std::unique_ptr<llvm::DenseMap<const Stmt *, CounterPair>> RegionCounterMap; std::unique_ptr<llvm::DenseMap<const Stmt *, uint64_t>> StmtCountMap; std::unique_ptr<llvm::InstrProfRecord> ProfRecord; std::unique_ptr<MCDC::State> RegionMCDCState; @@ -110,6 +110,7 @@ class CodeGenPGO { bool canEmitMCDCCoverage(const CGBuilderTy &Builder); public: + std::pair<bool, bool> getIsCounterPair(const Stmt *S) const; void emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, llvm::Value *StepV); void emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder, const Expr *S, @@ -122,6 +123,18 @@ class CodeGenPGO { Address MCDCCondBitmapAddr, llvm::Value *Val, CodeGenFunction &CGF); + void markStmtAsUsed(bool Skipped, const Stmt *S) { + // Do nothing. + } + + void markStmtMaybeUsed(const Stmt *S) { + // Do nothing. + } + + void verifyCounterMap() const { + // Do nothing. + } + /// Return the region count for the counter at the given index. uint64_t getRegionCount(const Stmt *S) { if (!RegionCounterMap) @@ -130,7 +143,7 @@ class CodeGenPGO { return 0; // With profiles from a diff ering version of clang we can have mismatched // decl counts. Don't crash in such a case. - auto Index = (*RegionCounterMap)[S]; + auto Index = (*RegionCounterMap)[S].Executed; if (Index >= RegionCounts.size()) return 0; return RegionCounts[Index]; diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index dfffa12b639f24..f09157771d2b5c 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -882,7 +882,7 @@ struct CounterCoverageMappingBuilder : public CoverageMappingBuilder, public ConstStmtVisitor<CounterCoverageMappingBuilder> { /// The map of statements to count values. - llvm::DenseMap<const Stmt *, unsigned> &CounterMap; + llvm::DenseMap<const Stmt *, CounterPair> &CounterMap; MCDC::State &MCDCState; @@ -931,7 +931,7 @@ struct CounterCoverageMappingBuilder /// /// This should only be called on statements that have a dedicated counter. Counter getRegionCounter(const Stmt *S) { - return Counter::getCounter(CounterMap[S]); + return Counter::getCounter(CounterMap[S].Executed); } struct BranchCounterPair { @@ -1457,7 +1457,7 @@ struct CounterCoverageMappingBuilder CounterCoverageMappingBuilder( CoverageMappingModuleGen &CVM, - llvm::DenseMap<const Stmt *, unsigned> &CounterMap, + llvm::DenseMap<const Stmt *, CounterPair> &CounterMap, MCDC::State &MCDCState, SourceManager &SM, const LangOptions &LangOpts) : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap), MCDCState(MCDCState), MCDCBuilder(CVM.getCodeGenModule(), MCDCState) {} diff --git a/clang/lib/CodeGen/CoverageMappingGen.h b/clang/lib/CodeGen/CoverageMappingGen.h index fe4b93f3af8561..0ed50597e1dc3e 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.h +++ b/clang/lib/CodeGen/CoverageMappingGen.h @@ -95,6 +95,7 @@ class CoverageSourceInfo : public PPCallbacks, namespace CodeGen { class CodeGenModule; +class CounterPair; namespace MCDC { struct State; @@ -158,7 +159,7 @@ class CoverageMappingGen { CoverageMappingModuleGen &CVM; SourceManager &SM; const LangOptions &LangOpts; - llvm::DenseMap<const Stmt *, unsigned> *CounterMap; + llvm::DenseMap<const Stmt *, CounterPair> *CounterMap; MCDC::State *MCDCState; public: @@ -169,7 +170,7 @@ class CoverageMappingGen { CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM, const LangOptions &LangOpts, - llvm::DenseMap<const Stmt *, unsigned> *CounterMap, + llvm::DenseMap<const Stmt *, CounterPair> *CounterMap, MCDC::State *MCDCState) : CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(CounterMap), MCDCState(MCDCState) {} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
