[Mlir-commits] [clang] [llvm] [mlir] [clang][lldb][mlir] Fix some identical sub-expressions warnings (NFC) (PR #95715)

Shivam Gupta llvmlistbot at llvm.org
Fri Jul 26 05:02:53 PDT 2024


https://github.com/xgupta updated https://github.com/llvm/llvm-project/pull/95715

>From c30bfb621f413c7271bb5a02a7d699e68e053ba1 Mon Sep 17 00:00:00 2001
From: Shivam Gupta <shivam98.tkg at gmail.com>
Date: Sun, 16 Jun 2024 23:39:47 +0530
Subject: [PATCH 1/4] [clang][lldb][mlir] Fix some identical sub-expressions
 warnings (NFC)

    This is actually reported in https://pvs-studio.com/en/blog/posts/cpp/1126/, fragment N4-8

    V501 There are identical sub-expressions 'FirstStmt->getStmtClass()' to the left and to the right of the '!=' operator. ASTUtils.cpp:99
    V501 There are identical sub-expressions to the left and to the right of the '!=' operator: Fn1->isVariadic() != Fn1->isVariadic(). SemaOverload.cpp:10190
    V501 There are identical sub-expressions to the left and to the right of the '<' operator: G1->Rank < G1->Rank. SCCIterator.h:285
    V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:581
    V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:583
    V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:585
    V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:646
    V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:648
    V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:650
    V501 There are identical sub-expressions 'EltRange.getEnd() >= Range.getEnd()' to the left and to the right of the '||' operator. HTMLLogger.cpp:421
    V501 There are identical sub-expressions 'SrcExpr.get()->containsErrors()' to the left and to the right of the '||' operator. SemaCast.cpp:2938
    V501 There are identical sub-expressions 'ND->getDeclContext()' to the left and to the right of the '!=' operator. SemaDeclCXX.cpp:4391
    V501 There are identical sub-expressions 'DepType != OMPC_DOACROSS_source' to the left and to the right of the '&&' operator. SemaOpenMP.cpp:24348
    V501 There are identical sub-expressions '!OldMethod->isStatic()' to the left and to the right of the '&&' operator. SemaOverload.cpp:1425
    V501 There are identical sub-expressions 'lldb::eTypeClassUnion' to the left and to the right of the '|' operator. JSONUtils.cpp:139
    V501 There are identical sub-expressions to the left and to the right of the '&&' operator: !BFI &&!BFI. JumpThreading.cpp:2531
    V501 There are identical sub-expressions 'BI->isConditional()' to the left and to the right of the '&&' operator. VPlanHCFGBuilder.cpp:401
    V501 There are identical sub-expressions to the left and to the right of the '==' operator: getNumRows() == getNumRows(). Simplex.cpp:108
---
 clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp    |  7 +++----
 clang/lib/Sema/SemaCast.cpp                        |  3 +--
 clang/lib/Sema/SemaDeclCXX.cpp                     |  2 +-
 clang/lib/Sema/SemaOpenMP.cpp                      |  3 +--
 llvm/lib/Transforms/Scalar/JumpThreading.cpp       |  2 +-
 llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp |  2 +-
 mlir/lib/Analysis/Presburger/Simplex.cpp           |  2 +-
 mlir/lib/Interfaces/ValueBoundsOpInterface.cpp     | 12 ++++++------
 8 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index a36cb41a63dfb..323f9698dc2aa 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -430,11 +430,10 @@ class HTMLLogger : public Logger {
               AST.getSourceManager(), AST.getLangOpts());
           if (EltRange.isInvalid())
             continue;
-          if (EltRange.getBegin() < Range.getBegin() ||
-              EltRange.getEnd() >= Range.getEnd() ||
-              EltRange.getEnd() < Range.getBegin() ||
-              EltRange.getEnd() >= Range.getEnd())
+          if (EltRange.getEnd() <= Range.getBegin() ||
+              EltRange.getBegin() >= Range.getEnd())
             continue;
+
           unsigned Off = EltRange.getBegin().getRawEncoding() -
                          Range.getBegin().getRawEncoding();
           unsigned Len = EltRange.getEnd().getRawEncoding() -
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index eca8363ee9605..18de4f6007d87 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2945,8 +2945,7 @@ void CastOperation::CheckCStyleCast() {
   if (Self.getASTContext().isDependenceAllowed() &&
       (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
        SrcExpr.get()->isValueDependent())) {
-    assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||
-            SrcExpr.get()->containsErrors()) &&
+    assert((DestType->containsErrors() || SrcExpr.get()->containsErrors()) &&
            "should only occur in error-recovery path.");
     assert(Kind == CK_Dependent);
     return;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 1cca8ac9b9343..9a953186d9521 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4239,7 +4239,7 @@ bool Sema::DiagRedefinedPlaceholderFieldDecl(SourceLocation Loc,
   Diag(Loc, diag::err_using_placeholder_variable) << Name;
   for (DeclContextLookupResult::iterator It = Found; It != Result.end(); It++) {
     const NamedDecl *ND = *It;
-    if (ND->getDeclContext() != ND->getDeclContext())
+    if (ND->getDeclContext() != ClassDecl->getDeclContext())
       break;
     if (isa<FieldDecl, IndirectFieldDecl>(ND) &&
         ND->isPlaceholderVar(getLangOpts()))
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 9c80b3eec914c..1d378e6b830ae 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -23087,8 +23087,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPDoacrossClause(
   if (DSAStack->getCurrentDirective() == OMPD_ordered &&
       DepType != OMPC_DOACROSS_source && DepType != OMPC_DOACROSS_sink &&
       DepType != OMPC_DOACROSS_sink_omp_cur_iteration &&
-      DepType != OMPC_DOACROSS_source_omp_cur_iteration &&
-      DepType != OMPC_DOACROSS_source) {
+      DepType != OMPC_DOACROSS_source_omp_cur_iteration) {
     Diag(DepLoc, diag::err_omp_unexpected_clause_value)
         << "'source' or 'sink'" << getOpenMPClauseName(OMPC_doacross);
     return nullptr;
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 7a0b661a07799..a2ffab9fbe7ce 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2520,7 +2520,7 @@ void JumpThreadingPass::updateBlockFreqAndEdgeWeight(BasicBlock *PredBB,
                                                      BlockFrequencyInfo *BFI,
                                                      BranchProbabilityInfo *BPI,
                                                      bool HasProfile) {
-  assert(((BFI && BPI) || (!BFI && !BFI)) &&
+  assert(((BFI && BPI) || (!BFI && !BPI)) &&
          "Both BFI & BPI should either be set or unset");
 
   if (!BFI) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 6e633739fcc3d..2868e0130f8b6 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -412,7 +412,7 @@ void PlainCFGBuilder::buildPlainCFG() {
                                 : static_cast<VPBlockBase *>(Successor));
       continue;
     }
-    assert(BI->isConditional() && NumSuccs == 2 && BI->isConditional() &&
+    assert(BI->isConditional() && NumSuccs == 2 &&
            "block must have conditional branch with 2 successors");
     // Look up the branch condition to get the corresponding VPValue
     // representing the condition bit in VPlan (which may be in another VPBB).
diff --git a/mlir/lib/Analysis/Presburger/Simplex.cpp b/mlir/lib/Analysis/Presburger/Simplex.cpp
index 7c8a019557132..43e9727cf75c8 100644
--- a/mlir/lib/Analysis/Presburger/Simplex.cpp
+++ b/mlir/lib/Analysis/Presburger/Simplex.cpp
@@ -106,8 +106,8 @@ Simplex::Unknown &SimplexBase::unknownFromRow(unsigned row) {
 
 unsigned SimplexBase::addZeroRow(bool makeRestricted) {
   // Resize the tableau to accommodate the extra row.
+  unsigned oldNumRows = getNumRows();
   unsigned newRow = tableau.appendExtraRow();
-  assert(getNumRows() == getNumRows() && "Inconsistent tableau size");
   rowUnknown.emplace_back(~con.size());
   con.emplace_back(Orientation::Row, makeRestricted, newRow);
   undoLog.emplace_back(UndoLogEntry::RemoveLastConstraint);
diff --git a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
index 6420c192b257d..4b9f026832044 100644
--- a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
+++ b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
@@ -778,11 +778,11 @@ FailureOr<bool>
 ValueBoundsConstraintSet::areOverlappingSlices(MLIRContext *ctx,
                                                HyperrectangularSlice slice1,
                                                HyperrectangularSlice slice2) {
-  assert(slice1.getMixedOffsets().size() == slice1.getMixedOffsets().size() &&
+  assert(slice1.getMixedOffsets().size() == slice2.getMixedOffsets().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedSizes().size() == slice1.getMixedSizes().size() &&
+  assert(slice1.getMixedSizes().size() == slice2.getMixedSizes().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedStrides().size() == slice1.getMixedStrides().size() &&
+  assert(slice1.getMixedStrides().size() == slice2.getMixedStrides().size() &&
          "expected slices of same rank");
 
   Builder b(ctx);
@@ -843,11 +843,11 @@ FailureOr<bool>
 ValueBoundsConstraintSet::areEquivalentSlices(MLIRContext *ctx,
                                               HyperrectangularSlice slice1,
                                               HyperrectangularSlice slice2) {
-  assert(slice1.getMixedOffsets().size() == slice1.getMixedOffsets().size() &&
+  assert(slice1.getMixedOffsets().size() == slice2.getMixedOffsets().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedSizes().size() == slice1.getMixedSizes().size() &&
+  assert(slice1.getMixedSizes().size() == slice2.getMixedSizes().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedStrides().size() == slice1.getMixedStrides().size() &&
+  assert(slice1.getMixedStrides().size() == slice2.getMixedStrides().size() &&
          "expected slices of same rank");
 
   // The two slices are equivalent if all of their offsets, sizes and strides

>From 9143422b8b7b6c8919b1d5833b1fc60477463022 Mon Sep 17 00:00:00 2001
From: Shivam Gupta <shivam98.tkg at gmail.com>
Date: Tue, 18 Jun 2024 00:18:29 +0530
Subject: [PATCH 2/4] fix the assertion failure in select.ll test case

---
 llvm/lib/Transforms/Scalar/JumpThreading.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index a2ffab9fbe7ce..25d092739e4b0 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2380,7 +2380,10 @@ void JumpThreadingPass::threadEdge(BasicBlock *BB,
   // Build BPI/BFI before any changes are made to IR.
   bool HasProfile = doesBlockHaveProfileData(BB);
   auto *BFI = getOrCreateBFI(HasProfile);
-  auto *BPI = getOrCreateBPI(BFI != nullptr);
+  BranchProbabilityInfo *BPI = nullptr;
+  if (BFI) {
+    BPI = getOrCreateBPI(true);
+  }
 
   // And finally, do it!  Start by factoring the predecessors if needed.
   BasicBlock *PredBB;

>From 6b6bb70fda46e8885e04556548f7e8901f275e8a Mon Sep 17 00:00:00 2001
From: Shivam Gupta <shivam98.tkg at gmail.com>
Date: Tue, 18 Jun 2024 00:21:21 +0530
Subject: [PATCH 3/4] removed assertion from Simplex.cpp as per review

---
 mlir/lib/Analysis/Presburger/Simplex.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mlir/lib/Analysis/Presburger/Simplex.cpp b/mlir/lib/Analysis/Presburger/Simplex.cpp
index 43e9727cf75c8..254904d69f75a 100644
--- a/mlir/lib/Analysis/Presburger/Simplex.cpp
+++ b/mlir/lib/Analysis/Presburger/Simplex.cpp
@@ -106,7 +106,6 @@ Simplex::Unknown &SimplexBase::unknownFromRow(unsigned row) {
 
 unsigned SimplexBase::addZeroRow(bool makeRestricted) {
   // Resize the tableau to accommodate the extra row.
-  unsigned oldNumRows = getNumRows();
   unsigned newRow = tableau.appendExtraRow();
   rowUnknown.emplace_back(~con.size());
   con.emplace_back(Orientation::Row, makeRestricted, newRow);

>From 3d328b6c0d57a78e2d418aa91fd07e43013fc9df Mon Sep 17 00:00:00 2001
From: xgupta <shivma98.tkg at gmail.com>
Date: Fri, 26 Jul 2024 14:02:20 +0200
Subject: [PATCH 4/4] resolve review comment

---
 clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp | 2 +-
 clang/lib/Sema/SemaDeclCXX.cpp                  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index 323f9698dc2aa..8de61bb102326 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -430,7 +430,7 @@ class HTMLLogger : public Logger {
               AST.getSourceManager(), AST.getLangOpts());
           if (EltRange.isInvalid())
             continue;
-          if (EltRange.getEnd() <= Range.getBegin() ||
+          if (EltRange.getEnd() < Range.getBegin() ||
               EltRange.getBegin() >= Range.getEnd())
             continue;
 
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9a953186d9521..1cca8ac9b9343 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4239,7 +4239,7 @@ bool Sema::DiagRedefinedPlaceholderFieldDecl(SourceLocation Loc,
   Diag(Loc, diag::err_using_placeholder_variable) << Name;
   for (DeclContextLookupResult::iterator It = Found; It != Result.end(); It++) {
     const NamedDecl *ND = *It;
-    if (ND->getDeclContext() != ClassDecl->getDeclContext())
+    if (ND->getDeclContext() != ND->getDeclContext())
       break;
     if (isa<FieldDecl, IndirectFieldDecl>(ND) &&
         ND->isPlaceholderVar(getLangOpts()))



More information about the Mlir-commits mailing list