[llvm-branch-commits] [clang] [MC/DC] Prune MCDCLogOpStack and use CGF.isMCDCDecisionExpr (PR #125410)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Feb 2 05:34:05 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: NAKAMURA Takumi (chapuni)
<details>
<summary>Changes</summary>
`MCDCLogOpStack` is used only for detection of the Decision root. It can be detected with `MCDC::State::DecisionByStmt`.
---
Full diff: https://github.com/llvm/llvm-project/pull/125410.diff
4 Files Affected:
- (modified) clang/lib/CodeGen/CGExprScalar.cpp (+14-26)
- (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+4-12)
- (modified) clang/lib/CodeGen/CodeGenFunction.h (+5-3)
- (modified) clang/lib/CodeGen/CodeGenPGO.h (+14)
``````````diff
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,
``````````
</details>
https://github.com/llvm/llvm-project/pull/125410
More information about the llvm-branch-commits
mailing list