[llvm-branch-commits] [clang] [MC/DC] Enable usage of `!` among `&&` and `||` (PR #125406)

NAKAMURA Takumi via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Sun Feb 2 05:29:04 PST 2025


https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/125406

In the current implementation, `!(a || b) && c` was not treated as one Decision with three terms.

Fixes #124563

>From f2cf50e10b59d7d461967baef4d589c9282d0f6d Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 2 Feb 2025 22:11:51 +0900
Subject: [PATCH] [MC/DC] Enable usage of `!` among `&&` and `||`

In the current implementation, `!(a || b) && c` was not treated as one
Decision with three terms.

Fixes #124563
---
 clang/include/clang/AST/IgnoreExpr.h          |  8 ++
 clang/lib/CodeGen/CodeGenFunction.cpp         | 12 ++-
 clang/lib/CodeGen/CodeGenPGO.cpp              |  8 +-
 clang/lib/CodeGen/CoverageMappingGen.cpp      | 12 +++
 .../test/CoverageMapping/mcdc-nested-expr.cpp |  6 +-
 clang/test/Profile/c-mcdc-not.c               | 95 +++++++++++++++++++
 6 files changed, 132 insertions(+), 9 deletions(-)

diff --git a/clang/include/clang/AST/IgnoreExpr.h b/clang/include/clang/AST/IgnoreExpr.h
index 917bada61fa6fd..c48c0c0daf8151 100644
--- a/clang/include/clang/AST/IgnoreExpr.h
+++ b/clang/include/clang/AST/IgnoreExpr.h
@@ -134,6 +134,14 @@ inline Expr *IgnoreElidableImplicitConstructorSingleStep(Expr *E) {
   return E;
 }
 
+inline Expr *IgnoreUOpLNotSingleStep(Expr *E) {
+  if (auto *UO = dyn_cast<UnaryOperator>(E)) {
+    if (UO->getOpcode() == UO_LNot)
+      return UO->getSubExpr();
+  }
+  return E;
+}
+
 inline Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) {
   if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
     return ICE->getSubExprAsWritten();
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index bbef277a524480..2c380ac926b1e7 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/Basic/Builtins.h"
@@ -1748,12 +1749,13 @@ bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond,
 
 /// Strip parentheses and simplistic logical-NOT operators.
 const Expr *CodeGenFunction::stripCond(const Expr *C) {
-  while (const UnaryOperator *Op = dyn_cast<UnaryOperator>(C->IgnoreParens())) {
-    if (Op->getOpcode() != UO_LNot)
-      break;
-    C = Op->getSubExpr();
+  while (true) {
+    const Expr *SC =
+        IgnoreExprNodes(C, IgnoreParensSingleStep, IgnoreUOpLNotSingleStep);
+    if (C == SC)
+      return SC;
+    C = SC;
   }
-  return C->IgnoreParens();
 }
 
 /// Determine whether the given condition is an instrumentable condition
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 792373839107f0..0fd49b880bba30 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -247,8 +247,9 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
     }
 
     if (const Expr *E = dyn_cast<Expr>(S)) {
-      const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(E->IgnoreParens());
-      if (BinOp && BinOp->isLogicalOp()) {
+      if (const auto *BinOp =
+              dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
+          BinOp && BinOp->isLogicalOp()) {
         /// Check for "split-nested" logical operators. This happens when a new
         /// boolean expression logical-op nest is encountered within an existing
         /// boolean expression, separated by a non-logical operator.  For
@@ -280,7 +281,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
       return true;
 
     if (const Expr *E = dyn_cast<Expr>(S)) {
-      const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(E->IgnoreParens());
+      const BinaryOperator *BinOp =
+          dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
       if (BinOp && BinOp->isLogicalOp()) {
         assert(LogOpStack.back() == BinOp);
         LogOpStack.pop_back();
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index f09157771d2b5c..9bf73cf27a5fa9 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -799,6 +799,12 @@ struct MCDCCoverageBuilder {
   /// Return the LHS Decision ([0,0] if not set).
   const mcdc::ConditionIDs &back() const { return DecisionStack.back(); }
 
+  void swapConds() {
+    if (DecisionStack.empty())
+      return;
+
+    std::swap(DecisionStack.back()[false], DecisionStack.back()[true]);
+  }
   /// Push the binary operator statement to track the nest level and assign IDs
   /// to the operator's LHS and RHS.  The RHS may be a larger subtree that is
   /// broken up on successive levels.
@@ -2241,6 +2247,12 @@ struct CounterCoverageMappingBuilder
             SM.isInSystemHeader(SM.getSpellingLoc(E->getEndLoc())));
   }
 
+  void VisitUnaryLNot(const UnaryOperator *E) {
+    MCDCBuilder.swapConds();
+    Visit(E->getSubExpr());
+    MCDCBuilder.swapConds();
+  }
+
   void VisitBinLAnd(const BinaryOperator *E) {
     if (isExprInSystemHeader(E)) {
       LeafExprSet.insert(E);
diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp
index bb82873e3b600d..a49dad0b336126 100644
--- a/clang/test/CoverageMapping/mcdc-nested-expr.cpp
+++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp
@@ -29,6 +29,10 @@ bool func_expect(bool a, bool b, bool c) {
 // Doesn't split exprs.
 // CHECK: func_lnot{{.*}}:
 bool func_lnot(bool a, bool b, bool c, bool d) {
-  // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
   return !(a || b) && !(c && d);
+  // CHECK:  Decision,File 0, [[@LINE-1]]:10 -> [[#L:@LINE-1]]:32 = M:5, C:4
+  // CHECK:  Branch,File 0, [[#L]]:12 -> [[#L]]:13 = (#0 - #2), #2 [1,0,3]
+  // CHECK:  Branch,File 0, [[#L]]:17 -> [[#L]]:18 = (#2 - #3), #3 [3,0,2]
+  // CHECK:  Branch,File 0, [[#L]]:25 -> [[#L]]:26 = #4, (#1 - #4) [2,4,0]
+  // CHECK:  Branch,File 0, [[#L]]:30 -> [[#L]]:31 = #5, (#4 - #5) [4,0,0]
 }
diff --git a/clang/test/Profile/c-mcdc-not.c b/clang/test/Profile/c-mcdc-not.c
index 7083aa226fb952..d6d79bf15cb086 100644
--- a/clang/test/Profile/c-mcdc-not.c
+++ b/clang/test/Profile/c-mcdc-not.c
@@ -85,3 +85,98 @@ int test(int a, int b, int c, int d, int e, int f) {
 // MCDC:  %[[BITS:.+]] = load i8, ptr %[[LAB4]], align 1
 // MCDC:  %[[LAB8:[0-9]+]] = or i8 %[[BITS]], %[[LAB7]]
 // MCDC:  store i8 %[[LAB8]], ptr %[[LAB4]], align 1
+
+int internot(int a, int b, int c, int d, int e, int f) {
+  return !(!(!a && b) || !(!!(!c && d) || !(e && !f)));
+}
+
+// MCDC-LABEL: @internot(
+// MCDC-DAG:  store i32 0, ptr %mcdc.addr, align 4
+
+// Branch #2, (#0 - #2) [1,3,0]
+// !a [+0 => b][+6 => END]
+// MCDC-DAG:  %[[A:.+]] = load i32, ptr %a.addr, align 4
+// MCDC-DAG:  %[[AB:.+]] = icmp ne i32 %[[A]], 0
+// MCDC-DAG:  %[[AN:.+]] = xor i1 %[[AB]], true
+// MCDC-DAG:  %[[M1:.+]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG:  %[[M1T:.+]] = add i32 %[[M1]], 0
+// MCDC-DAG:  %[[M1F:.+]] = add i32 %[[M1]], 6
+// MCDC-DAG:  %[[M1S:.+]] = select i1 %[[AN]], i32 %[[M1T]], i32 %[[M1F]]
+// MCDC-DAG:  store i32 %[[M1S]], ptr %mcdc.addr, align 4
+
+// Branch #3, (#2 - #3) [3,2,0]
+// b [+0 => c][+7 => END]
+// MCDC-DAG:  %[[B:.+]] = load i32, ptr %b.addr, align 4
+// MCDC-DAG:  %[[BB:.+]] = icmp ne i32 %[[B]], 0
+// MCDC-DAG:  %[[M3:.+]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG:  %[[M3T:.+]] = add i32 %[[M3]], 0
+// MCDC-DAG:  %[[M3F:.+]] = add i32 %[[M3]], 7
+// MCDC-DAG:  %[[M3S:.+]] = select i1 %[[BB]], i32 %[[M3T]], i32 %[[M3F]]
+// MCDC-DAG:  store i32 %[[M3S]], ptr %mcdc.addr, align 4
+
+// Branch #5, (#1 - #5) [2,5,4]
+// !c [+0 => d][+0 => e]
+// MCDC-DAG:  %[[C:.+]] = load i32, ptr %c.addr, align 4
+// MCDC-DAG:  %[[CB:.+]] = icmp ne i32 %[[C]], 0
+// MCDC-DAG:  %[[CN:.+]] = xor i1 %[[CB]], true
+// MCDC-DAG:  %[[M2:.+]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG:  %[[M2T:.+]] = add i32 %[[M2]], 0
+// MCDC-DAG:  %[[M2F:.+]] = add i32 %[[M2]], 0
+// MCDC-DAG:  %[[M2S:.+]] = select i1 %[[CN]], i32 %[[M2T]], i32 %[[M2F]]
+// MCDC-DAG:  store i32 %[[M2S]], ptr %mcdc.addr, align 4
+
+// Branch #6, (#5 - #6) [5,0,4]
+// d [+8 => END][+1 => e]]
+// MCDC-DAG:  %[[D:.+]] = load i32, ptr %d.addr, align 4
+// MCDC-DAG:  %[[DB:.+]] = icmp ne i32 %[[D]], 0
+// MCDC-DAG:  %[[M5:.+]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG:  %[[M5T:.+]] = add i32 %[[M5]], 8
+// MCDC-DAG:  %[[M5F:.+]] = add i32 %[[M5]], 1
+// MCDC-DAG:  %[[M5S:.+]] = select i1 %[[DB]], i32 %[[M5T]], i32 %[[M5F]]
+// MCDC-DAG:  store i32 %[[M5S]], ptr %mcdc.addr, align 4
+
+// Branch #7, (#4 - #7) [4,6,0]
+// e [+0 => f][+0 => END]
+// from:
+//   [c => +0]
+//   [d => +1]
+// MCDC-DAG:  %[[E:.+]] = load i32, ptr %e.addr, align 4
+// MCDC-DAG:  %[[EB:.+]] = icmp ne i32 %[[E]], 0
+// MCDC-DAG:  %[[M4:.+]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG:  %[[M4T:.+]] = add i32 %[[M4]], 0
+// MCDC-DAG:  %[[M4F:.+]] = add i32 %[[M4]], 0
+// MCDC-DAG:  %[[M4S:.+]] = select i1 %[[EB]], i32 %[[M4T]], i32 %[[M4F]]
+// MCDC-DAG:  store i32 %[[M4S]], ptr %mcdc.addr, align 4
+
+// Branch #8, (#7 - #8) [6,0,0]
+// !f [+4 => END][+2 => END]
+// MCDC-DAG:  %[[F:.+]] = load i32, ptr %f.addr, align 4
+// MCDC-DAG:  %[[FB:.+]] = icmp ne i32 %[[F]], 0
+// MCDC-DAG:  %[[FN:.+]] = xor i1 %[[FB]], true
+// MCDC-DAG:  %[[M6:.+]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG:  %[[M6T:.+]] = add i32 %[[M6]], 4
+// MCDC-DAG:  %[[M6F:.+]] = add i32 %[[M6]], 2
+// MCDC-DAG:  %[[M6S:.+]] = select i1 %[[FN]], i32 %[[M6T]], i32 %[[M6F]]
+// MCDC-DAG:  store i32 %[[M6S]], ptr %mcdc.addr, align 4
+
+// from:
+//   [e => +0]
+//   [f => +2]
+//     [c => +0]
+//     [d => +1]
+//   [f => +4]
+//     [c => +0]
+//     [d => +1]
+//   [a => +6]
+//   [b => +7]
+//   [d => +8]
+// MCDC-DAG:  %[[T0:.+]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG:  %[[T:.+]] = add i32 %[[T0]], 0
+// MCDC-DAG:  %[[TA:.+]] = lshr i32 %[[T]], 3
+// MCDC-DAG:  %[[BA:.+]] = getelementptr inbounds i8, ptr @__profbm_internot, i32 %[[TA]]
+// MCDC-DAG:  %[[BI:.+]] = and i32 %[[T]], 7
+// MCDC-DAG:  %[[BI1:.+]] = trunc i32 %[[BI]] to i8
+// MCDC-DAG:  %[[BM:.+]] = shl i8 1, %[[BI1]]
+// MCDC-DAG:  %mcdc.bits = load i8, ptr %[[BA]], align 1
+// MCDC-DAG:  %[[BN:.+]] = or i8 %mcdc.bits, %[[BM]]
+// MCDC-DAG:  store i8 %[[BN]], ptr %[[BA]], align 1



More information about the llvm-branch-commits mailing list