release/18.x: [clang][CodeGen] Keep processing the rest of AST after encountering unsupported MC/DC expressions (#82464) #82866
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@evodius96 What do you think about merging this PR to the release branch? |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen Author: None (llvmbot) ChangesBackport d4bfca3 Requested by: @whentojump Full diff: https://github.com/llvm/llvm-project/pull/82866.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 5d7c3847745762..fb4e86e8bd8053 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -240,9 +240,12 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
if (MCDCMaxCond == 0)
return true;
- /// At the top of the logical operator nest, reset the number of conditions.
- if (LogOpStack.empty())
+ /// At the top of the logical operator nest, reset the number of conditions,
+ /// also forget previously seen split nesting cases.
+ if (LogOpStack.empty()) {
NumCond = 0;
+ SplitNestedLogicalOp = false;
+ }
if (const Expr *E = dyn_cast<Expr>(S)) {
const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(E->IgnoreParens());
@@ -293,7 +296,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
"contains an operation with a nested boolean expression. "
"Expression will not be covered");
Diag.Report(S->getBeginLoc(), DiagID);
- return false;
+ return true;
}
/// Was the maximum number of conditions encountered?
@@ -304,7 +307,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
"number of conditions (%0) exceeds max (%1). "
"Expression will not be covered");
Diag.Report(S->getBeginLoc(), DiagID) << NumCond << MCDCMaxCond;
- return false;
+ return true;
}
// Otherwise, allocate the number of bytes required for the bitmap
|
evodius96
approved these changes
Feb 24, 2024
…nsupported MC/DC expressions (llvm#82464) Currently, upon seeing unsupported decisions (more than 6 conditions, or split nesting), the post-visitor hook dataTraverseStmtPost() returns a false. As a result, in the rest of tree even supported decisions will be skipped as well. Like in the below code: { // CompoundStmt a && b; // 1: BinaryOperator (supported) a && foo(b && c); // 2: BinaryOperator (not yet supported due to split // nesting) a && b; // 3: BinaryOperator (supported) } Decision 3 will not be processed at all. And only one "Decision" region will be emitted. Compiler explorer example: https://godbolt.org/z/Px61sesoo We hope to process such cases and emit two "Decision" regions (1 and 3) in the above example. (cherry picked from commit d4bfca3)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport d4bfca3
Requested by: @whentojump