Skip to content

Conversation

@abhinavgaba
Copy link
Contributor

No description provided.

@abhinavgaba abhinavgaba marked this pull request as ready for review September 25, 2025 17:48
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. clang:openmp OpenMP related changes to Clang labels Sep 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-clang

Author: Abhinav Gaba (abhinavgaba)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/160746.diff

1 Files Affected:

  • (modified) clang/lib/CodeGen/CGOpenMPRuntime.cpp (+12-13)
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 6c68311daf0ba..c90e1a487daf9 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6804,7 +6804,7 @@ class MappableExprsHandler {
   /// they were computed by collectAttachPtrExprInfo(), if they are semantically
   /// different.
   struct AttachPtrExprComparator {
-    const MappableExprsHandler *Handler;
+    const MappableExprsHandler *Handler = nullptr;
     // Cache of previous equality comparison results.
     mutable llvm::DenseMap<std::pair<const Expr *, const Expr *>, bool>
         CachedEqualityComparisons;
@@ -6817,8 +6817,8 @@ class MappableExprsHandler {
         return false;
 
       // First, compare by complexity (depth)
-      auto ItLHS = Handler->AttachPtrComponentDepthMap.find(LHS);
-      auto ItRHS = Handler->AttachPtrComponentDepthMap.find(RHS);
+      const auto ItLHS = Handler->AttachPtrComponentDepthMap.find(LHS);
+      const auto ItRHS = Handler->AttachPtrComponentDepthMap.find(RHS);
 
       std::optional<size_t> DepthLHS =
           (ItLHS != Handler->AttachPtrComponentDepthMap.end()) ? ItLHS->second
@@ -6856,7 +6856,7 @@ class MappableExprsHandler {
     /// results, if available, otherwise does a recursive semantic comparison.
     bool areEqual(const Expr *LHS, const Expr *RHS) const {
       // Check cache first for faster lookup
-      auto CachedResultIt = CachedEqualityComparisons.find({LHS, RHS});
+      const auto CachedResultIt = CachedEqualityComparisons.find({LHS, RHS});
       if (CachedResultIt != CachedEqualityComparisons.end())
         return CachedResultIt->second;
 
@@ -7142,7 +7142,7 @@ class MappableExprsHandler {
   const Expr *getAttachPtrExpr(
       OMPClauseMappableExprCommon::MappableExprComponentListRef Components)
       const {
-    auto It = AttachPtrExprMap.find(Components);
+    const auto It = AttachPtrExprMap.find(Components);
     if (It != AttachPtrExprMap.end())
       return It->second;
 
@@ -8478,8 +8478,9 @@ class MappableExprsHandler {
     } else if (auto *ME = dyn_cast<MemberExpr>(PointerExpr)) {
       AttachPtrAddr = CGF.EmitMemberExpr(ME).getAddress();
     } else if (auto *UO = dyn_cast<UnaryOperator>(PointerExpr)) {
-      if (UO->getOpcode() == UO_Deref)
-        AttachPtrAddr = CGF.EmitLValue(UO).getAddress();
+      assert(UO->getOpcode() == UO_Deref &&
+             "Unexpected unary-operator on attach-ptr-expr");
+      AttachPtrAddr = CGF.EmitLValue(UO).getAddress();
     }
     assert(AttachPtrAddr.isValid() &&
            "Failed to get address for attach pointer expression");
@@ -8524,12 +8525,10 @@ class MappableExprsHandler {
 
     // Pointer attachment is needed at map-entering time or for declare
     // mappers.
-    if (!isa<const OMPDeclareMapperDecl *>(CurDir) &&
-        !isOpenMPTargetMapEnteringDirective(
-            cast<const OMPExecutableDirective *>(CurDir)->getDirectiveKind()))
-      return false;
-
-    return true;
+    return isa<const OMPDeclareMapperDecl *>(CurDir) ||
+           isOpenMPTargetMapEnteringDirective(
+               cast<const OMPExecutableDirective *>(CurDir)
+                   ->getDirectiveKind());
   }
 
   /// Computes the attach-ptr expr for \p Components, and updates various maps

@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-clang-codegen

Author: Abhinav Gaba (abhinavgaba)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/160746.diff

1 Files Affected:

  • (modified) clang/lib/CodeGen/CGOpenMPRuntime.cpp (+12-13)
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 6c68311daf0ba..c90e1a487daf9 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6804,7 +6804,7 @@ class MappableExprsHandler {
   /// they were computed by collectAttachPtrExprInfo(), if they are semantically
   /// different.
   struct AttachPtrExprComparator {
-    const MappableExprsHandler *Handler;
+    const MappableExprsHandler *Handler = nullptr;
     // Cache of previous equality comparison results.
     mutable llvm::DenseMap<std::pair<const Expr *, const Expr *>, bool>
         CachedEqualityComparisons;
@@ -6817,8 +6817,8 @@ class MappableExprsHandler {
         return false;
 
       // First, compare by complexity (depth)
-      auto ItLHS = Handler->AttachPtrComponentDepthMap.find(LHS);
-      auto ItRHS = Handler->AttachPtrComponentDepthMap.find(RHS);
+      const auto ItLHS = Handler->AttachPtrComponentDepthMap.find(LHS);
+      const auto ItRHS = Handler->AttachPtrComponentDepthMap.find(RHS);
 
       std::optional<size_t> DepthLHS =
           (ItLHS != Handler->AttachPtrComponentDepthMap.end()) ? ItLHS->second
@@ -6856,7 +6856,7 @@ class MappableExprsHandler {
     /// results, if available, otherwise does a recursive semantic comparison.
     bool areEqual(const Expr *LHS, const Expr *RHS) const {
       // Check cache first for faster lookup
-      auto CachedResultIt = CachedEqualityComparisons.find({LHS, RHS});
+      const auto CachedResultIt = CachedEqualityComparisons.find({LHS, RHS});
       if (CachedResultIt != CachedEqualityComparisons.end())
         return CachedResultIt->second;
 
@@ -7142,7 +7142,7 @@ class MappableExprsHandler {
   const Expr *getAttachPtrExpr(
       OMPClauseMappableExprCommon::MappableExprComponentListRef Components)
       const {
-    auto It = AttachPtrExprMap.find(Components);
+    const auto It = AttachPtrExprMap.find(Components);
     if (It != AttachPtrExprMap.end())
       return It->second;
 
@@ -8478,8 +8478,9 @@ class MappableExprsHandler {
     } else if (auto *ME = dyn_cast<MemberExpr>(PointerExpr)) {
       AttachPtrAddr = CGF.EmitMemberExpr(ME).getAddress();
     } else if (auto *UO = dyn_cast<UnaryOperator>(PointerExpr)) {
-      if (UO->getOpcode() == UO_Deref)
-        AttachPtrAddr = CGF.EmitLValue(UO).getAddress();
+      assert(UO->getOpcode() == UO_Deref &&
+             "Unexpected unary-operator on attach-ptr-expr");
+      AttachPtrAddr = CGF.EmitLValue(UO).getAddress();
     }
     assert(AttachPtrAddr.isValid() &&
            "Failed to get address for attach pointer expression");
@@ -8524,12 +8525,10 @@ class MappableExprsHandler {
 
     // Pointer attachment is needed at map-entering time or for declare
     // mappers.
-    if (!isa<const OMPDeclareMapperDecl *>(CurDir) &&
-        !isOpenMPTargetMapEnteringDirective(
-            cast<const OMPExecutableDirective *>(CurDir)->getDirectiveKind()))
-      return false;
-
-    return true;
+    return isa<const OMPDeclareMapperDecl *>(CurDir) ||
+           isOpenMPTargetMapEnteringDirective(
+               cast<const OMPExecutableDirective *>(CurDir)
+                   ->getDirectiveKind());
   }
 
   /// Computes the attach-ptr expr for \p Components, and updates various maps

@abhinavgaba abhinavgaba merged commit 39ed57c into llvm:main Sep 25, 2025
15 checks passed
@abhinavgaba abhinavgaba deleted the nfc-review-feedback-from-155625 branch September 25, 2025 18:13
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:codegen IR generation bugs: mangling, exceptions, etc. clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants