https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/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::first` is used. Each counter number can have `None` to suppress emitting counter increment. `second` is initialized as `None` by default, since most `Stmt*` don't have a pair of counters. This change also provides stubs for the verifyer. 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 >From e4172ca273a6fdfcbfc4662c9e37276ef34c2df4 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Thu, 17 Oct 2024 00:32:26 +0900 Subject: [PATCH] Introduce the type `CounterPair` for RegionCounterMap `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::first` is used. Each counter number can have `None` to suppress emitting counter increment. `second` is initialized as `None` by default, since most `Stmt*` don't have a pair of counters. This change also provides stubs for the verifyer. 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 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 --- clang/lib/CodeGen/CGDecl.cpp | 9 ++++++++- clang/lib/CodeGen/CGExpr.cpp | 1 + clang/lib/CodeGen/CGExprScalar.cpp | 9 +++++++-- clang/lib/CodeGen/CGStmt.cpp | 3 +++ clang/lib/CodeGen/CodeGenFunction.cpp | 3 +++ clang/lib/CodeGen/CodeGenFunction.h | 6 ++++++ clang/lib/CodeGen/CodeGenModule.h | 19 +++++++++++++++++++ clang/lib/CodeGen/CodeGenPGO.cpp | 14 ++++++++++---- clang/lib/CodeGen/CodeGenPGO.h | 17 +++++++++++++++-- clang/lib/CodeGen/CoverageMappingGen.cpp | 6 +++--- clang/lib/CodeGen/CoverageMappingGen.h | 5 +++-- 11 files changed, 78 insertions(+), 14 deletions(-) diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 563f728e29d781..ed5f41b624b62b 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -362,6 +362,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()); @@ -1869,7 +1871,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(); } @@ -1978,6 +1983,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 52d2f6d52abf94..2fd6b02a3395ee 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5134,6 +5134,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 b7f5b932c56b6f..74e93f889f4261 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -4982,8 +4982,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. @@ -5122,8 +5124,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. @@ -5247,6 +5251,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 41dc91c578c800..dbc1ce9bf993cd 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; } @@ -845,6 +846,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) { RunCleanupsScope ExecutedScope(*this); EmitStmt(Executed); } + PGO.markStmtMaybeUsed(Skipped); return; } } @@ -2170,6 +2172,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 24723e392c2a3a..371aa494e014bd 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1606,6 +1606,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()) @@ -1728,6 +1730,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 9ba0ed02a564dd..89ac3b342d0a7c 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1620,6 +1620,12 @@ class CodeGenFunction : public CodeGenTypeCache { uint64_t LoopCount) const; public: + std::pair<bool, bool> getIsCounterPair(const Stmt *S) const { + return PGO.getIsCounterPair(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 c58bb88035ca8a..9dc497321b42a6 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -101,6 +101,25 @@ enum ForDefinition_t : bool { ForDefinition = true }; +class CounterPair : public std::pair<uint32_t, uint32_t> { +private: + static constexpr uint32_t None = (1u << 31); /// None is set + +public: + static constexpr uint32_t Mask = None - 1; + +public: + CounterPair(unsigned Val = 0) { + assert(!(Val & ~Mask)); + first = Val; + second = None; + } + + std::pair<bool, bool> getIsCounterPair() const { + return {!(first & None), !(second & None)}; + } +}; + struct OrderGlobalInitsOrStermFinalizers { unsigned int priority; unsigned int lex_order; diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 820bb521ccf850..069469e3de856b 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -164,7 +164,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. @@ -175,7 +175,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), @@ -1084,7 +1084,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()); @@ -1186,12 +1186,18 @@ CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader *PGOReader, Fn->setEntryCount(FunctionCount); } +std::pair<bool, bool> CodeGenPGO::getIsCounterPair(const Stmt *S) const { + if (!RegionCounterMap || RegionCounterMap->count(S) == 0) + return {false, false}; + return (*RegionCounterMap)[S].getIsCounterPair(); +} + void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, llvm::Value *StepV) { if (!RegionCounterMap || !Builder.GetInsertBlock()) return; - unsigned Counter = (*RegionCounterMap)[S]; + unsigned Counter = (*RegionCounterMap)[S].first; // 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..83f35785e5327d 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() { + // 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 differing version of clang we can have mismatched // decl counts. Don't crash in such a case. - auto Index = (*RegionCounterMap)[S]; + auto Index = (*RegionCounterMap)[S].first; if (Index >= RegionCounts.size()) return 0; return RegionCounts[Index]; diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 07015834bc84f3..08e4ac80e0e87c 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -885,7 +885,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; @@ -938,7 +938,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].first); } /// Push a region onto the stack. @@ -1421,7 +1421,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
