https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/113113
>From 16e2bb8b73bcde1c2618bb358a905a9f463c1217 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Sun, 20 Oct 2024 16:24:26 +0900 Subject: [PATCH 1/2] [Coverage][Single] Enable Branch coverage for `BinLAnd` and `BinLOr` --- clang/lib/CodeGen/CGExprScalar.cpp | 83 +++++++++++++++++++----- clang/lib/CodeGen/CGStmt.cpp | 4 -- clang/lib/CodeGen/CodeGenFunction.cpp | 43 ++++++++++-- clang/lib/CodeGen/CoverageMappingGen.cpp | 6 -- 4 files changed, 104 insertions(+), 32 deletions(-) diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 11d4ec8a267605..83962ba96aa484 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -4918,6 +4918,9 @@ Value *ScalarExprEmitter::VisitBinAssign(const BinaryOperator *E) { } Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { + auto HasLHSSkip = CGF.getIsCounterPair(E); + auto HasRHSSkip = CGF.getIsCounterPair(E->getRHS()); + // Perform vector logical and on comparisons with zero vectors. if (E->getType()->isVectorType()) { CGF.incrementProfileCounter(E); @@ -4964,11 +4967,17 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { CodeGenFunction::isInstrumentedCondition(E->getRHS())) { CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond); llvm::BasicBlock *FBlock = CGF.createBasicBlock("land.end"); + llvm::BasicBlock *RHSSkip = + (HasRHSSkip.second ? CGF.createBasicBlock("land.rhsskip") : FBlock); llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("land.rhscnt"); - Builder.CreateCondBr(RHSCond, RHSBlockCnt, FBlock); + Builder.CreateCondBr(RHSCond, RHSBlockCnt, RHSSkip); CGF.EmitBlock(RHSBlockCnt); - CGF.incrementProfileCounter(E->getRHS()); + CGF.incrementProfileCounter(false, E->getRHS()); CGF.EmitBranch(FBlock); + if (HasRHSSkip.second) { + CGF.EmitBlock(RHSSkip); + CGF.incrementProfileCounter(true, E->getRHS()); + } CGF.EmitBlock(FBlock); } @@ -4997,12 +5006,21 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { llvm::BasicBlock *ContBlock = CGF.createBasicBlock("land.end"); llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("land.rhs"); + llvm::BasicBlock *LHSFalseBlock = + (HasLHSSkip.second ? CGF.createBasicBlock("land.lhsskip") : ContBlock); + CodeGenFunction::ConditionalEvaluation eval(CGF); // Branch on the LHS first. If it is false, go to the failure (cont) block. - CGF.EmitBranchOnBoolExpr(E->getLHS(), RHSBlock, ContBlock, + CGF.EmitBranchOnBoolExpr(E->getLHS(), RHSBlock, LHSFalseBlock, CGF.getProfileCount(E->getRHS())); + if (HasLHSSkip.second) { + CGF.EmitBlock(LHSFalseBlock); + CGF.incrementProfileCounter(true, E); + CGF.EmitBranch(ContBlock); + } + // Any edges into the ContBlock are now from an (indeterminate number of) // edges from this first condition. All of these values will be false. Start // setting up the PHI node in the Cont Block for this. @@ -5014,7 +5032,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { eval.begin(CGF); CGF.EmitBlock(RHSBlock); - CGF.incrementProfileCounter(E); + CGF.incrementProfileCounter(false, E); Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS()); eval.end(CGF); @@ -5024,15 +5042,24 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { // If we're generating for profiling or coverage, generate a branch on the // RHS to a block that increments the RHS true counter needed to track branch // condition coverage. + llvm::BasicBlock *ContIncoming = RHSBlock; if (InstrumentRegions && CodeGenFunction::isInstrumentedCondition(E->getRHS())) { CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond); llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("land.rhscnt"); - Builder.CreateCondBr(RHSCond, RHSBlockCnt, ContBlock); + llvm::BasicBlock *RHSBlockSkip = + (HasRHSSkip.second ? CGF.createBasicBlock("land.rhsskip") : ContBlock); + Builder.CreateCondBr(RHSCond, RHSBlockCnt, RHSBlockSkip); CGF.EmitBlock(RHSBlockCnt); - CGF.incrementProfileCounter(E->getRHS()); + CGF.incrementProfileCounter(false, E->getRHS()); CGF.EmitBranch(ContBlock); PN->addIncoming(RHSCond, RHSBlockCnt); + if (HasRHSSkip.second) { + CGF.EmitBlock(RHSBlockSkip); + CGF.incrementProfileCounter(true, E->getRHS()); + CGF.EmitBranch(ContBlock); + ContIncoming = RHSBlockSkip; + } } // Emit an unconditional branch from this block to ContBlock. @@ -5042,7 +5069,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { CGF.EmitBlock(ContBlock); } // Insert an entry into the phi node for the edge with the value of RHSCond. - PN->addIncoming(RHSCond, RHSBlock); + PN->addIncoming(RHSCond, ContIncoming); CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. @@ -5060,6 +5087,9 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { } Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { + auto HasLHSSkip = CGF.getIsCounterPair(E); + auto HasRHSSkip = CGF.getIsCounterPair(E->getRHS()); + // Perform vector logical or on comparisons with zero vectors. if (E->getType()->isVectorType()) { CGF.incrementProfileCounter(E); @@ -5088,7 +5118,7 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { bool LHSCondVal; if (CGF.ConstantFoldsToSimpleInteger(E->getLHS(), LHSCondVal)) { if (!LHSCondVal) { // If we have 0 || X, just emit X. - CGF.incrementProfileCounter(E); + CGF.incrementProfileCounter(false, E); // If the top of the logical operator nest, reset the MCDC temp to 0. if (CGF.MCDCLogOpStack.empty()) @@ -5106,11 +5136,17 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { CodeGenFunction::isInstrumentedCondition(E->getRHS())) { CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond); llvm::BasicBlock *FBlock = CGF.createBasicBlock("lor.end"); + llvm::BasicBlock *RHSSkip = + (HasRHSSkip.second ? CGF.createBasicBlock("lor.rhsskip") : FBlock); llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("lor.rhscnt"); - Builder.CreateCondBr(RHSCond, FBlock, RHSBlockCnt); + Builder.CreateCondBr(RHSCond, RHSSkip, RHSBlockCnt); CGF.EmitBlock(RHSBlockCnt); - CGF.incrementProfileCounter(E->getRHS()); + CGF.incrementProfileCounter(false, E->getRHS()); CGF.EmitBranch(FBlock); + if (HasRHSSkip.second) { + CGF.EmitBlock(RHSSkip); + CGF.incrementProfileCounter(true, E->getRHS()); + } CGF.EmitBlock(FBlock); } @@ -5138,14 +5174,22 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { llvm::BasicBlock *ContBlock = CGF.createBasicBlock("lor.end"); llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("lor.rhs"); + llvm::BasicBlock *LHSTrueBlock = + (HasLHSSkip.second ? CGF.createBasicBlock("lor.lhsskip") : ContBlock); CodeGenFunction::ConditionalEvaluation eval(CGF); // Branch on the LHS first. If it is true, go to the success (cont) block. - CGF.EmitBranchOnBoolExpr(E->getLHS(), ContBlock, RHSBlock, + CGF.EmitBranchOnBoolExpr(E->getLHS(), LHSTrueBlock, RHSBlock, CGF.getCurrentProfileCount() - CGF.getProfileCount(E->getRHS())); + if (HasLHSSkip.second) { + CGF.EmitBlock(LHSTrueBlock); + CGF.incrementProfileCounter(true, E); + CGF.EmitBranch(ContBlock); + } + // Any edges into the ContBlock are now from an (indeterminate number of) // edges from this first condition. All of these values will be true. Start // setting up the PHI node in the Cont Block for this. @@ -5159,7 +5203,7 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { // Emit the RHS condition as a bool value. CGF.EmitBlock(RHSBlock); - CGF.incrementProfileCounter(E); + CGF.incrementProfileCounter(false, E); Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS()); eval.end(CGF); @@ -5170,21 +5214,30 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { // If we're generating for profiling or coverage, generate a branch on the // RHS to a block that increments the RHS true counter needed to track branch // condition coverage. + llvm::BasicBlock *ContIncoming = RHSBlock; if (InstrumentRegions && CodeGenFunction::isInstrumentedCondition(E->getRHS())) { CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond); llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("lor.rhscnt"); - Builder.CreateCondBr(RHSCond, ContBlock, RHSBlockCnt); + llvm::BasicBlock *RHSTrueBlock = + (HasRHSSkip.second ? CGF.createBasicBlock("lor.rhsskip") : ContBlock); + Builder.CreateCondBr(RHSCond, RHSTrueBlock, RHSBlockCnt); CGF.EmitBlock(RHSBlockCnt); - CGF.incrementProfileCounter(E->getRHS()); + CGF.incrementProfileCounter(false, E->getRHS()); CGF.EmitBranch(ContBlock); PN->addIncoming(RHSCond, RHSBlockCnt); + if (HasRHSSkip.second) { + CGF.EmitBlock(RHSTrueBlock); + CGF.incrementProfileCounter(true, E->getRHS()); + CGF.EmitBranch(ContBlock); + ContIncoming = RHSTrueBlock; + } } // Emit an unconditional branch from this block to ContBlock. Insert an entry // into the phi node for the edge with the value of RHSCond. CGF.EmitBlock(ContBlock); - PN->addIncoming(RHSCond, RHSBlock); + PN->addIncoming(RHSCond, ContIncoming); CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index e28048d0ec4d90..ee42560b8870dc 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -43,10 +43,6 @@ using namespace CodeGen; // Statement Emission //===----------------------------------------------------------------------===// -namespace llvm { -extern cl::opt<bool> EnableSingleByteCoverage; -} // namespace llvm - void CodeGenFunction::EmitStopPoint(const Stmt *S) { if (CGDebugInfo *DI = getDebugInfo()) { SourceLocation Loc; diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index fcd225b0dc7f45..7f3f4bdbdbbc1d 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1762,6 +1762,7 @@ void CodeGenFunction::EmitBranchToCounterBlock( return EmitBranchOnBoolExpr(Cond, TrueBlock, FalseBlock, TrueCount, LH); const Stmt *CntrStmt = (CntrIdx ? CntrIdx : Cond); + auto HasSkip = getIsCounterPair(CntrStmt); llvm::BasicBlock *ThenBlock = nullptr; llvm::BasicBlock *ElseBlock = nullptr; @@ -1770,6 +1771,10 @@ void CodeGenFunction::EmitBranchToCounterBlock( // Create the block we'll use to increment the appropriate counter. llvm::BasicBlock *CounterIncrBlock = createBasicBlock("lop.rhscnt"); + llvm::BasicBlock *SkipIncrBlock = + (HasSkip.second ? createBasicBlock("lop.rhsskip") : nullptr); + llvm::BasicBlock *SkipNextBlock = nullptr; + // Set block pointers according to Logical-AND (BO_LAnd) semantics. This // means we need to evaluate the condition and increment the counter on TRUE: // @@ -1783,8 +1788,9 @@ void CodeGenFunction::EmitBranchToCounterBlock( // goto TrueBlock; if (LOp == BO_LAnd) { + SkipNextBlock = FalseBlock; ThenBlock = CounterIncrBlock; - ElseBlock = FalseBlock; + ElseBlock = (SkipIncrBlock ? SkipIncrBlock : SkipNextBlock); NextBlock = TrueBlock; } @@ -1801,7 +1807,8 @@ void CodeGenFunction::EmitBranchToCounterBlock( // goto FalseBlock; else if (LOp == BO_LOr) { - ThenBlock = TrueBlock; + SkipNextBlock = TrueBlock; + ThenBlock = (SkipIncrBlock ? SkipIncrBlock : SkipNextBlock); ElseBlock = CounterIncrBlock; NextBlock = FalseBlock; } else { @@ -1811,11 +1818,17 @@ void CodeGenFunction::EmitBranchToCounterBlock( // Emit Branch based on condition. EmitBranchOnBoolExpr(Cond, ThenBlock, ElseBlock, TrueCount, LH); + if (SkipIncrBlock) { + EmitBlock(SkipIncrBlock); + incrementProfileCounter(true, CntrStmt); + EmitBranch(SkipNextBlock); + } + // Emit the block containing the counter increment(s). EmitBlock(CounterIncrBlock); // Increment corresponding counter; if index not provided, use Cond as index. - incrementProfileCounter(CntrStmt); + incrementProfileCounter(false, CntrStmt); // Go to the next block. EmitBranch(NextBlock); @@ -1834,6 +1847,8 @@ void CodeGenFunction::EmitBranchOnBoolExpr( Cond = Cond->IgnoreParens(); if (const BinaryOperator *CondBOp = dyn_cast<BinaryOperator>(Cond)) { + auto HasSkip = getIsCounterPair(CondBOp); + // Handle X && Y in a condition. if (CondBOp->getOpcode() == BO_LAnd) { MCDCLogOpStack.push_back(CondBOp); @@ -1865,6 +1880,8 @@ void CodeGenFunction::EmitBranchOnBoolExpr( // Emit the LHS as a conditional. If the LHS conditional is false, we // want to jump to the FalseBlock. llvm::BasicBlock *LHSTrue = createBasicBlock("land.lhs.true"); + llvm::BasicBlock *LHSFalse = + (HasSkip.second ? createBasicBlock("land.lhsskip") : FalseBlock); // The counter tells us how often we evaluate RHS, and all of TrueCount // can be propagated to that branch. uint64_t RHSCount = getProfileCount(CondBOp->getRHS()); @@ -1875,12 +1892,17 @@ void CodeGenFunction::EmitBranchOnBoolExpr( // Propagate the likelihood attribute like __builtin_expect // __builtin_expect(X && Y, 1) -> X and Y are likely // __builtin_expect(X && Y, 0) -> only Y is unlikely - EmitBranchOnBoolExpr(CondBOp->getLHS(), LHSTrue, FalseBlock, RHSCount, + EmitBranchOnBoolExpr(CondBOp->getLHS(), LHSTrue, LHSFalse, RHSCount, LH == Stmt::LH_Unlikely ? Stmt::LH_None : LH); + if (HasSkip.second) { + EmitBlock(LHSFalse); + incrementProfileCounter(true, CondBOp); + EmitBranch(FalseBlock); + } EmitBlock(LHSTrue); } - incrementProfileCounter(CondBOp); + incrementProfileCounter(false, CondBOp); setCurrentProfileCount(getProfileCount(CondBOp->getRHS())); // Any temporaries created here are conditional. @@ -1920,6 +1942,8 @@ void CodeGenFunction::EmitBranchOnBoolExpr( } // Emit the LHS as a conditional. If the LHS conditional is true, we // want to jump to the TrueBlock. + llvm::BasicBlock *LHSTrue = + (HasSkip.second ? createBasicBlock("lor.lhsskip") : TrueBlock); llvm::BasicBlock *LHSFalse = createBasicBlock("lor.lhs.false"); // We have the count for entry to the RHS and for the whole expression // being true, so we can divy up True count between the short circuit and @@ -1934,12 +1958,17 @@ void CodeGenFunction::EmitBranchOnBoolExpr( // __builtin_expect(X || Y, 1) -> only Y is likely // __builtin_expect(X || Y, 0) -> both X and Y are unlikely ApplyDebugLocation DL(*this, Cond); - EmitBranchOnBoolExpr(CondBOp->getLHS(), TrueBlock, LHSFalse, LHSCount, + EmitBranchOnBoolExpr(CondBOp->getLHS(), LHSTrue, LHSFalse, LHSCount, LH == Stmt::LH_Likely ? Stmt::LH_None : LH); + if (HasSkip.second) { + EmitBlock(LHSTrue); + incrementProfileCounter(true, CondBOp); + EmitBranch(TrueBlock); + } EmitBlock(LHSFalse); } - incrementProfileCounter(CondBOp); + incrementProfileCounter(false, CondBOp); setCurrentProfileCount(getProfileCount(CondBOp->getRHS())); // Any temporaries created here are conditional. diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 6abf0b333b246b..9179410c706217 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -2185,9 +2185,6 @@ struct CounterCoverageMappingBuilder extendRegion(E->getRHS()); propagateCounts(getRegionCounter(E), E->getRHS()); - if (llvm::EnableSingleByteCoverage) - return; - // Track RHS True/False Decision. const auto DecisionRHS = MCDCBuilder.back(); @@ -2246,9 +2243,6 @@ struct CounterCoverageMappingBuilder extendRegion(E->getRHS()); propagateCounts(getRegionCounter(E), E->getRHS()); - if (llvm::EnableSingleByteCoverage) - return; - // Track RHS True/False Decision. const auto DecisionRHS = MCDCBuilder.back(); >From ad997c2f539ab2622e61c106bb3d646930777712 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Wed, 23 Oct 2024 14:39:35 +0000 Subject: [PATCH 2/2] Fix cases when LHS is skipped --- clang/lib/CodeGen/CGExprScalar.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index de85d4ad63833e..fd67622fe81548 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -4949,7 +4949,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { bool LHSCondVal; if (CGF.ConstantFoldsToSimpleInteger(E->getLHS(), LHSCondVal)) { if (LHSCondVal) { // If we have 1 && X, just emit X. - CGF.incrementProfileCounter(E); + CGF.incrementProfileCounter(false, E, true); // If the top of the logical operator nest, reset the MCDC temp to 0. if (CGF.MCDCLogOpStack.empty()) @@ -4993,7 +4993,12 @@ 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())) { + CGF.markStmtAsUsed(false, E); + if (HasLHSSkip.second) + CGF.incrementProfileCounter(true, E); + CGF.markStmtMaybeUsed(E->getRHS()); + return llvm::Constant::getNullValue(ResTy); } } @@ -5119,7 +5124,7 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { bool LHSCondVal; if (CGF.ConstantFoldsToSimpleInteger(E->getLHS(), LHSCondVal)) { if (!LHSCondVal) { // If we have 0 || X, just emit X. - CGF.incrementProfileCounter(false, E); + CGF.incrementProfileCounter(false, E, true); // If the top of the logical operator nest, reset the MCDC temp to 0. if (CGF.MCDCLogOpStack.empty()) @@ -5163,7 +5168,12 @@ 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())) { + CGF.markStmtAsUsed(false, E); + if (HasLHSSkip.second) + CGF.incrementProfileCounter(true, E); + CGF.markStmtMaybeUsed(E->getRHS()); + return llvm::ConstantInt::get(ResTy, 1); } } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits