https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/125410
`MCDCLogOpStack` is used only for detection of the Decision root. It can be detected with `MCDC::State::DecisionByStmt`. >From 8eff226c98bdcfcd1366120699a42e0c4c73375c Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Sun, 2 Feb 2025 22:00:48 +0900 Subject: [PATCH] [MC/DC] Prune MCDCLogOpStack and use CGF.isMCDCDecisionExpr `MCDCLogOpStack` is used only for detection of the Decision root. It can be detected with `MCDC::State::DecisionByStmt`. --- clang/lib/CodeGen/CGExprScalar.cpp | 40 ++++++++++----------------- clang/lib/CodeGen/CodeGenFunction.cpp | 16 +++-------- clang/lib/CodeGen/CodeGenFunction.h | 8 ++++-- clang/lib/CodeGen/CodeGenPGO.h | 14 ++++++++++ 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index df850421c72c6c..37b32977b8bd3e 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -4990,11 +4990,9 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { CGF.incrementProfileCounter(E); // If the top of the logical operator nest, reset the MCDC temp to 0. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeResetMCDCCondBitmap(E); - CGF.MCDCLogOpStack.push_back(E); - Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS()); // If we're generating for profiling or coverage, generate a branch to a @@ -5014,9 +5012,8 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { } else CGF.markStmtMaybeUsed(E->getRHS()); - CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeUpdateMCDCTestVectorBitmap(E); // ZExt result to int or bool. @@ -5031,11 +5028,9 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { } // If the top of the logical operator nest, reset the MCDC temp to 0. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeResetMCDCCondBitmap(E); - CGF.MCDCLogOpStack.push_back(E); - llvm::BasicBlock *ContBlock = CGF.createBasicBlock("land.end"); llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("land.rhs"); @@ -5086,9 +5081,8 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { // Insert an entry into the phi node for the edge with the value of RHSCond. PN->addIncoming(RHSCond, RHSBlock); - CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeUpdateMCDCTestVectorBitmap(E); // Artificial location to preserve the scope information @@ -5133,11 +5127,9 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { CGF.incrementProfileCounter(E); // If the top of the logical operator nest, reset the MCDC temp to 0. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeResetMCDCCondBitmap(E); - CGF.MCDCLogOpStack.push_back(E); - Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS()); // If we're generating for profiling or coverage, generate a branch to a @@ -5157,9 +5149,8 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { } else CGF.markStmtMaybeUsed(E->getRHS()); - CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeUpdateMCDCTestVectorBitmap(E); // ZExt result to int or bool. @@ -5174,11 +5165,9 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { } // If the top of the logical operator nest, reset the MCDC temp to 0. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeResetMCDCCondBitmap(E); - CGF.MCDCLogOpStack.push_back(E); - llvm::BasicBlock *ContBlock = CGF.createBasicBlock("lor.end"); llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("lor.rhs"); @@ -5229,9 +5218,8 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { CGF.EmitBlock(ContBlock); PN->addIncoming(RHSCond, RHSBlock); - CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. - if (CGF.MCDCLogOpStack.empty()) + if (CGF.isMCDCDecisionExpr(E)) CGF.maybeUpdateMCDCTestVectorBitmap(E); // ZExt result to int. @@ -5390,8 +5378,8 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { } // If the top of the logical operator nest, reset the MCDC temp to 0. - if (CGF.MCDCLogOpStack.empty()) - CGF.maybeResetMCDCCondBitmap(condExpr); + if (auto E = CGF.stripCond(condExpr); CGF.isMCDCDecisionExpr(E)) + CGF.maybeResetMCDCCondBitmap(E); llvm::BasicBlock *LHSBlock = CGF.createBasicBlock("cond.true"); llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("cond.false"); @@ -5406,8 +5394,8 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { // If the top of the logical operator nest, update the MCDC bitmap for the // ConditionalOperator prior to visiting its LHS and RHS blocks, since they // may also contain a boolean expression. - if (CGF.MCDCLogOpStack.empty()) - CGF.maybeUpdateMCDCTestVectorBitmap(condExpr); + if (auto E = CGF.stripCond(condExpr); CGF.isMCDCDecisionExpr(E)) + CGF.maybeUpdateMCDCTestVectorBitmap(E); if (llvm::EnableSingleByteCoverage) CGF.incrementProfileCounter(lhsExpr); @@ -5426,8 +5414,8 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { // If the top of the logical operator nest, update the MCDC bitmap for the // ConditionalOperator prior to visiting its LHS and RHS blocks, since they // may also contain a boolean expression. - if (CGF.MCDCLogOpStack.empty()) - CGF.maybeUpdateMCDCTestVectorBitmap(condExpr); + if (auto E = CGF.stripCond(condExpr); CGF.isMCDCDecisionExpr(E)) + CGF.maybeUpdateMCDCTestVectorBitmap(E); if (llvm::EnableSingleByteCoverage) CGF.incrementProfileCounter(rhsExpr); diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index bbef277a524480..3a6b97a8a226fa 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1851,8 +1851,6 @@ void CodeGenFunction::EmitBranchOnBoolExpr( if (const BinaryOperator *CondBOp = dyn_cast<BinaryOperator>(Cond)) { // Handle X && Y in a condition. if (CondBOp->getOpcode() == BO_LAnd) { - MCDCLogOpStack.push_back(CondBOp); - // If we have "1 && X", simplify the code. "0 && X" would have constant // folded if the case was simple enough. bool ConstantBool = false; @@ -1862,7 +1860,6 @@ void CodeGenFunction::EmitBranchOnBoolExpr( incrementProfileCounter(CondBOp); EmitBranchToCounterBlock(CondBOp->getRHS(), BO_LAnd, TrueBlock, FalseBlock, TrueCount, LH); - MCDCLogOpStack.pop_back(); return; } @@ -1873,7 +1870,6 @@ void CodeGenFunction::EmitBranchOnBoolExpr( // br(X && 1) -> br(X). EmitBranchToCounterBlock(CondBOp->getLHS(), BO_LAnd, TrueBlock, FalseBlock, TrueCount, LH, CondBOp); - MCDCLogOpStack.pop_back(); return; } @@ -1903,13 +1899,10 @@ void CodeGenFunction::EmitBranchOnBoolExpr( EmitBranchToCounterBlock(CondBOp->getRHS(), BO_LAnd, TrueBlock, FalseBlock, TrueCount, LH); eval.end(*this); - MCDCLogOpStack.pop_back(); return; } if (CondBOp->getOpcode() == BO_LOr) { - MCDCLogOpStack.push_back(CondBOp); - // If we have "0 || X", simplify the code. "1 || X" would have constant // folded if the case was simple enough. bool ConstantBool = false; @@ -1919,7 +1912,6 @@ void CodeGenFunction::EmitBranchOnBoolExpr( incrementProfileCounter(CondBOp); EmitBranchToCounterBlock(CondBOp->getRHS(), BO_LOr, TrueBlock, FalseBlock, TrueCount, LH); - MCDCLogOpStack.pop_back(); return; } @@ -1930,7 +1922,6 @@ void CodeGenFunction::EmitBranchOnBoolExpr( // br(X || 0) -> br(X). EmitBranchToCounterBlock(CondBOp->getLHS(), BO_LOr, TrueBlock, FalseBlock, TrueCount, LH, CondBOp); - MCDCLogOpStack.pop_back(); return; } // Emit the LHS as a conditional. If the LHS conditional is true, we @@ -1963,7 +1954,6 @@ void CodeGenFunction::EmitBranchOnBoolExpr( RHSCount, LH); eval.end(*this); - MCDCLogOpStack.pop_back(); return; } } @@ -2048,7 +2038,7 @@ void CodeGenFunction::EmitBranchOnBoolExpr( // If not at the top of the logical operator nest, update MCDC temp with the // boolean result of the evaluated condition. - if (!MCDCLogOpStack.empty()) { + { const Expr *MCDCBaseExpr = Cond; // When a nested ConditionalOperator (ternary) is encountered in a boolean // expression, MC/DC tracks the result of the ternary, and this is tied to @@ -2058,7 +2048,9 @@ void CodeGenFunction::EmitBranchOnBoolExpr( if (ConditionalOp) MCDCBaseExpr = ConditionalOp; - maybeUpdateMCDCCondBitmap(MCDCBaseExpr, CondV); + if (isMCDCBranchExpr(stripCond(MCDCBaseExpr)) && + !isMCDCDecisionExpr(stripCond(Cond))) + maybeUpdateMCDCCondBitmap(MCDCBaseExpr, CondV); } llvm::MDNode *Weights = nullptr; diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index e978cad4336238..cac2c658addae5 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -312,9 +312,6 @@ class CodeGenFunction : public CodeGenTypeCache { /// nest would extend. SmallVector<llvm::CanonicalLoopInfo *, 4> OMPLoopNestStack; - /// Stack to track the Logical Operator recursion nest for MC/DC. - SmallVector<const BinaryOperator *, 16> MCDCLogOpStack; - /// Stack to track the controlled convergence tokens. SmallVector<llvm::ConvergenceControlInst *, 4> ConvergenceTokenStack; @@ -1679,6 +1676,11 @@ class CodeGenFunction : public CodeGenTypeCache { return (BOp && BOp->isLogicalOp()); } + bool isMCDCDecisionExpr(const Expr *E) const { + return PGO.isMCDCDecisionExpr(E); + } + bool isMCDCBranchExpr(const Expr *E) const { return PGO.isMCDCBranchExpr(E); } + /// Zero-init the MCDC temp value. void maybeResetMCDCCondBitmap(const Expr *E) { if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) { diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index 1944b640951d5c..0cbea643a65406 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -111,6 +111,20 @@ class CodeGenPGO { public: std::pair<bool, bool> getIsCounterPair(const Stmt *S) const; + + bool isMCDCDecisionExpr(const Expr *E) const { + if (!RegionMCDCState) + return false; + auto I = RegionMCDCState->DecisionByStmt.find(E); + if (I == RegionMCDCState->DecisionByStmt.end()) + return false; + return I->second.isValid(); + } + + bool isMCDCBranchExpr(const Expr *E) const { + return (RegionMCDCState && RegionMCDCState->BranchByStmt.contains(E)); + } + void emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, llvm::Value *StepV); void emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder, const Expr *S, _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits