Addressed review comments and made necessary modifications

Author: Chandra Ghale

clang/docs/OpenMPSupport.rst

@@ -405,8 +405,8 @@ implementation.
 | Extensions to depobj construct                              | :none:`unclaimed`         | :none:`unclaimed`         |                                                                          |
 +-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
 | Extensions to atomic construct                              | :none:`unclaimed`         | :none:`unclaimed`         |                                                                          |
-+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
-| Private reductions                                          | :none:`unclaimed`         | :none:`unclaimed`         |                                                                          |
++-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+ 
+| Private reductions                                          | :part:`partial`           | :none:`unclaimed`         | Parse/Sema:https://github.com/llvm/llvm-project/pull/129938              |
 +-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
 | Self maps                                                   | :none:`unclaimed`         | :none:`unclaimed`         |                                                                          |
 +-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+

clang/include/clang/AST/OpenMPClause.h

@@ -3658,14 +3658,18 @@ class OMPSharedClause final
 class OMPReductionClause final
     : public OMPVarListClause<OMPReductionClause>,
       public OMPClauseWithPostUpdate,
-      private llvm::TrailingObjects<OMPReductionClause, Expr *> {
+      private llvm::TrailingObjects<OMPReductionClause, Expr *, bool> {
   friend class OMPClauseReader;
   friend OMPVarListClause;
   friend TrailingObjects;
 
   /// Reduction modifier.
   OpenMPReductionClauseModifier Modifier = OMPC_REDUCTION_unknown;
 
+  /// Original Sharing  modifier.
+  OpenMPOriginalSharingModifier OriginalSharingModifier =
+      OMPC_ORIGINAL_SHARING_default;
+
   /// Reduction modifier location.
   SourceLocation ModifierLoc;
 
@@ -3678,9 +3682,6 @@ class OMPReductionClause final
   /// Name of custom operator.
   DeclarationNameInfo NameInfo;
 
-  /// Private variable reduction flag
-  llvm::SmallVector<bool, 8> IsPrivateVarReductionFlags;
-
   /// Build clause with number of variables \a N.
   ///
   /// \param StartLoc Starting location of the clause.
@@ -3694,12 +3695,14 @@ class OMPReductionClause final
   OMPReductionClause(SourceLocation StartLoc, SourceLocation LParenLoc,
                      SourceLocation ModifierLoc, SourceLocation ColonLoc,
                      SourceLocation EndLoc,
-                     OpenMPReductionClauseModifier Modifier, unsigned N,
-                     NestedNameSpecifierLoc QualifierLoc,
+                     OpenMPReductionClauseModifier Modifier,
+                     OpenMPOriginalSharingModifier OriginalSharingModifier,
+                     unsigned N, NestedNameSpecifierLoc QualifierLoc,
                      const DeclarationNameInfo &NameInfo)
       : OMPVarListClause<OMPReductionClause>(llvm::omp::OMPC_reduction,
                                              StartLoc, LParenLoc, EndLoc, N),
         OMPClauseWithPostUpdate(this), Modifier(Modifier),
+        OriginalSharingModifier(OriginalSharingModifier),
         ModifierLoc(ModifierLoc), ColonLoc(ColonLoc),
         QualifierLoc(QualifierLoc), NameInfo(NameInfo) {}
 
@@ -3715,6 +3718,11 @@ class OMPReductionClause final
   /// Sets reduction modifier.
   void setModifier(OpenMPReductionClauseModifier M) { Modifier = M; }
 
+  /// Sets Original Sharing  modifier.
+  void setOriginalSharingModifier(OpenMPOriginalSharingModifier M) {
+    OriginalSharingModifier = M;
+  }
+
   /// Sets location of the modifier.
   void setModifierLoc(SourceLocation Loc) { ModifierLoc = Loc; }
 
@@ -3760,6 +3768,31 @@ class OMPReductionClause final
   /// reduction copies.
   void setRHSExprs(ArrayRef<Expr *> RHSExprs);
 
+  /// Set the list private reduction flags
+  void setPrivateVariableReductionFlags(ArrayRef<bool> Flags) {
+    assert(Flags.size() == varlist_size() &&
+           "Number of private flags does not match vars");
+    std::copy(Flags.begin(), Flags.end(), getTrailingObjects<bool>());
+  }
+
+  /// Get the list of help private variable reduction flags
+  MutableArrayRef<bool> getPrivateVariableReductionFlags() {
+    return MutableArrayRef<bool>(getTrailingObjects<bool>(), varlist_size());
+  }
+  ArrayRef<bool> getPrivateVariableReductionFlags() const {
+    return ArrayRef<bool>(getTrailingObjects<bool>(), varlist_size());
+  }
+
+  /// Returns the number of Expr* objects in trailing storage
+  size_t numTrailingObjects(OverloadToken<Expr *>) const {
+    return varlist_size() * (Modifier == OMPC_REDUCTION_inscan ? 8 : 5);
+  }
+
+  /// Returns the number of bool flags in trailing storage
+  size_t numTrailingObjects(OverloadToken<bool>) const {
+    return varlist_size();
+  }
+
   /// Get the list of helper destination expressions.
   MutableArrayRef<Expr *> getRHSExprs() {
     return MutableArrayRef<Expr *>(getLHSExprs().end(), varlist_size());
@@ -3867,7 +3900,8 @@ class OMPReductionClause final
          ArrayRef<Expr *> LHSExprs, ArrayRef<Expr *> RHSExprs,
          ArrayRef<Expr *> ReductionOps, ArrayRef<Expr *> CopyOps,
          ArrayRef<Expr *> CopyArrayTemps, ArrayRef<Expr *> CopyArrayElems,
-         Stmt *PreInit, Expr *PostUpdate, ArrayRef<bool> IsPrivateVarReduction);
+         Stmt *PreInit, Expr *PostUpdate, ArrayRef<bool> IsPrivateVarReduction,
+         OpenMPOriginalSharingModifier OriginalSharingModifier);
 
   /// Creates an empty clause with the place for \a N variables.
   ///
@@ -3881,6 +3915,11 @@ class OMPReductionClause final
   /// Returns modifier.
   OpenMPReductionClauseModifier getModifier() const { return Modifier; }
 
+  /// Returns Original Sharing Modifier.
+  OpenMPOriginalSharingModifier getOriginalSharingModifier() const {
+    return OriginalSharingModifier;
+  }
+
   /// Returns modifier location.
   SourceLocation getModifierLoc() const { return ModifierLoc; }
 
@@ -3893,21 +3932,16 @@ class OMPReductionClause final
   /// Gets the nested name specifier.
   NestedNameSpecifierLoc getQualifierLoc() const { return QualifierLoc; }
 
-  /// Get all private variable reduction flags
-  ArrayRef<bool> getPrivateVariableReductionFlags() const {
-    return IsPrivateVarReductionFlags;
-  }
-
-  //// Set private variable reduction flags
-  void setPrivateVariableReductionFlags(ArrayRef<bool> Flags) {
-    std::copy(Flags.begin(), Flags.end(), IsPrivateVarReductionFlags.begin());
-  }
-
   using helper_expr_iterator = MutableArrayRef<Expr *>::iterator;
   using helper_expr_const_iterator = ArrayRef<const Expr *>::iterator;
   using helper_expr_range = llvm::iterator_range<helper_expr_iterator>;
   using helper_expr_const_range =
       llvm::iterator_range<helper_expr_const_iterator>;
+  using helper_flag_iterator = MutableArrayRef<bool>::iterator;
+  using helper_flag_const_iterator = ArrayRef<bool>::iterator;
+  using helper_flag_range = llvm::iterator_range<helper_flag_iterator>;
+  using helper_flag_const_range =
+      llvm::iterator_range<helper_flag_const_iterator>;
 
   helper_expr_const_range privates() const {
     return helper_expr_const_range(getPrivates().begin(), getPrivates().end());
@@ -3933,6 +3967,16 @@ class OMPReductionClause final
     return helper_expr_range(getRHSExprs().begin(), getRHSExprs().end());
   }
 
+  helper_flag_const_range private_var_reduction_flags() const {
+    return helper_flag_const_range(getPrivateVariableReductionFlags().begin(),
+                                   getPrivateVariableReductionFlags().end());
+  }
+
+  helper_flag_range private_var_reduction_flags() {
+    return helper_flag_range(getPrivateVariableReductionFlags().begin(),
+                             getPrivateVariableReductionFlags().end());
+  }
+
   helper_expr_const_range reduction_ops() const {
     return helper_expr_const_range(getReductionOps().begin(),
                                    getReductionOps().end());

clang/include/clang/Sema/SemaOpenMP.h

@@ -1162,6 +1162,12 @@ class SemaOpenMP : public SemaBase {
     SmallVector<SourceLocation, NumberOfOMPAllocateClauseModifiers>
         AllocClauseModifiersLoc;
     Expr *AllocateAlignment = nullptr;
+    struct OpenMPReductionClauseModifiers {
+      int ExtraModifier;
+      int OriginalSharingModifier;
+      OpenMPReductionClauseModifiers(int extra, int original)
+          : ExtraModifier(extra), OriginalSharingModifier(original) {}
+    };
   };
 
   OMPClause *ActOnOpenMPVarListClause(OpenMPClauseKind Kind,
@@ -1210,14 +1216,13 @@ class SemaOpenMP : public SemaBase {
                                      SourceLocation EndLoc);
   /// Called on well-formed 'reduction' clause.
   OMPClause *ActOnOpenMPReductionClause(
-      ArrayRef<Expr *> VarList, OpenMPReductionClauseModifier Modifier,
+      ArrayRef<Expr *> VarList,
+      OpenMPVarListDataTy::OpenMPReductionClauseModifiers Modifiers,
       SourceLocation StartLoc, SourceLocation LParenLoc,
       SourceLocation ModifierLoc, SourceLocation ColonLoc,
       SourceLocation EndLoc, CXXScopeSpec &ReductionIdScopeSpec,
       const DeclarationNameInfo &ReductionId,
-      ArrayRef<Expr *> UnresolvedReductions = {},
-      OpenMPOriginalSharingModifier OriginalShareModifier =
-          OMPC_ORIGINAL_SHARING_default);
+      ArrayRef<Expr *> UnresolvedReductions = {});
   /// Called on well-formed 'task_reduction' clause.
   OMPClause *ActOnOpenMPTaskReductionClause(
       ArrayRef<Expr *> VarList, SourceLocation StartLoc,

clang/lib/AST/OpenMPClause.cpp

@@ -798,12 +798,13 @@ OMPReductionClause *OMPReductionClause::Create(
     ArrayRef<Expr *> RHSExprs, ArrayRef<Expr *> ReductionOps,
     ArrayRef<Expr *> CopyOps, ArrayRef<Expr *> CopyArrayTemps,
     ArrayRef<Expr *> CopyArrayElems, Stmt *PreInit, Expr *PostUpdate,
-    ArrayRef<bool> IsPrivateVarReduction) {
-  void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(
-      (Modifier == OMPC_REDUCTION_inscan ? 8 : 5) * VL.size()));
-  auto *Clause = new (Mem)
-      OMPReductionClause(StartLoc, LParenLoc, ModifierLoc, EndLoc, ColonLoc,
-                         Modifier, VL.size(), QualifierLoc, NameInfo);
+    ArrayRef<bool> IsPrivateVarReduction,
+    OpenMPOriginalSharingModifier OrignalSharingModifier) {
+  void *Mem = C.Allocate(totalSizeToAlloc<Expr *, bool>(
+      (Modifier == OMPC_REDUCTION_inscan ? 8 : 5) * VL.size(), VL.size()));
+  auto *Clause = new (Mem) OMPReductionClause(
+      StartLoc, LParenLoc, ModifierLoc, EndLoc, ColonLoc, Modifier,
+      OrignalSharingModifier, VL.size(), QualifierLoc, NameInfo);
   Clause->setVarRefs(VL);
   Clause->setPrivates(Privates);
   Clause->setLHSExprs(LHSExprs);
@@ -830,8 +831,8 @@ OMPReductionClause *OMPReductionClause::Create(
 OMPReductionClause *
 OMPReductionClause::CreateEmpty(const ASTContext &C, unsigned N,
                                 OpenMPReductionClauseModifier Modifier) {
-  void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(
-      (Modifier == OMPC_REDUCTION_inscan ? 8 : 5) * N));
+  void *Mem = C.Allocate(totalSizeToAlloc<Expr *, bool>(
+      (Modifier == OMPC_REDUCTION_inscan ? 8 : 5) * N, N));
   auto *Clause = new (Mem) OMPReductionClause(N);
   Clause->setModifier(Modifier);
   return Clause;

clang/lib/Parse/ParseOpenMP.cpp

@@ -4693,7 +4693,10 @@ bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind DKind,
         return false;
       }
       ParenT.consumeClose();
-      assert(Tok.is(tok::comma) && "Expected comma.");
+      if (!Tok.is(tok::comma)) {
+        Diag(Tok.getLocation(), diag::err_expected) << "',' (comma)";
+        return false;
+      }
       (void)ConsumeToken();
     }
     ColonProtectionRAIIObject ColonRAII(*this);

clang/lib/Sema/SemaOpenMP.cpp

@@ -17390,10 +17390,11 @@ OMPClause *SemaOpenMP::ActOnOpenMPVarListClause(OpenMPClauseKind Kind,
     assert(0 <= ExtraModifier && ExtraModifier <= OMPC_REDUCTION_unknown &&
            "Unexpected lastprivate modifier.");
     Res = ActOnOpenMPReductionClause(
-        VarList, static_cast<OpenMPReductionClauseModifier>(ExtraModifier),
+        VarList,
+        OpenMPVarListDataTy::OpenMPReductionClauseModifiers(
+            ExtraModifier, OriginalSharingModifier),
         StartLoc, LParenLoc, ExtraModifierLoc, ColonLoc, EndLoc,
-        Data.ReductionOrMapperIdScopeSpec, Data.ReductionOrMapperId, {},
-        static_cast<OpenMPOriginalSharingModifier>(OriginalSharingModifier));
+        Data.ReductionOrMapperIdScopeSpec, Data.ReductionOrMapperId);
     break;
   case OMPC_task_reduction:
     Res = ActOnOpenMPTaskReductionClause(
@@ -19474,12 +19475,17 @@ static bool actOnOMPReductionKindClause(
 }
 
 OMPClause *SemaOpenMP::ActOnOpenMPReductionClause(
-    ArrayRef<Expr *> VarList, OpenMPReductionClauseModifier Modifier,
+    ArrayRef<Expr *> VarList,
+    OpenMPVarListDataTy::OpenMPReductionClauseModifiers Modifiers,
     SourceLocation StartLoc, SourceLocation LParenLoc,
     SourceLocation ModifierLoc, SourceLocation ColonLoc, SourceLocation EndLoc,
     CXXScopeSpec &ReductionIdScopeSpec, const DeclarationNameInfo &ReductionId,
-    ArrayRef<Expr *> UnresolvedReductions,
-    OpenMPOriginalSharingModifier OriginalSharingMod) {
+    ArrayRef<Expr *> UnresolvedReductions) {
+  OpenMPReductionClauseModifier Modifier =
+      static_cast<OpenMPReductionClauseModifier>(Modifiers.ExtraModifier);
+  OpenMPOriginalSharingModifier OriginalSharingModifier =
+      static_cast<OpenMPOriginalSharingModifier>(
+          Modifiers.OriginalSharingModifier);
   if (ModifierLoc.isValid() && Modifier == OMPC_REDUCTION_unknown) {
     Diag(LParenLoc, diag::err_omp_unexpected_clause_value)
         << getListOfPossibleValues(OMPC_reduction, /*First=*/0,
@@ -19501,7 +19507,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPReductionClause(
     Diag(ModifierLoc, diag::err_omp_wrong_inscan_reduction);
     return nullptr;
   }
-  ReductionData RD(VarList.size(), Modifier, OriginalSharingMod);
+  ReductionData RD(VarList.size(), Modifier, OriginalSharingModifier);
   if (actOnOMPReductionKindClause(SemaRef, DSAStack, OMPC_reduction, VarList,
                                   StartLoc, LParenLoc, ColonLoc, EndLoc,
                                   ReductionIdScopeSpec, ReductionId,
@@ -19515,7 +19521,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPReductionClause(
       RD.Privates, RD.LHSs, RD.RHSs, RD.ReductionOps, RD.InscanCopyOps,
       RD.InscanCopyArrayTemps, RD.InscanCopyArrayElems,
       buildPreInits(getASTContext(), RD.ExprCaptures),
-      buildPostUpdate(SemaRef, RD.ExprPostUpdates), RD.IsPrivateVarReduction);
+      buildPostUpdate(SemaRef, RD.ExprPostUpdates), RD.IsPrivateVarReduction,
+      OriginalSharingModifier);
 }
 
 OMPClause *SemaOpenMP::ActOnOpenMPTaskReductionClause(

clang/lib/Sema/TreeTransform.h

@@ -1913,14 +1913,16 @@ class TreeTransform {
   /// Subclasses may override this routine to provide different behavior.
   OMPClause *RebuildOMPReductionClause(
       ArrayRef<Expr *> VarList, OpenMPReductionClauseModifier Modifier,
+      OpenMPOriginalSharingModifier OriginalSharingModifier,
       SourceLocation StartLoc, SourceLocation LParenLoc,
       SourceLocation ModifierLoc, SourceLocation ColonLoc,
       SourceLocation EndLoc, CXXScopeSpec &ReductionIdScopeSpec,
       const DeclarationNameInfo &ReductionId,
       ArrayRef<Expr *> UnresolvedReductions) {
     return getSema().OpenMP().ActOnOpenMPReductionClause(
-        VarList, Modifier, StartLoc, LParenLoc, ModifierLoc, ColonLoc, EndLoc,
-        ReductionIdScopeSpec, ReductionId, UnresolvedReductions);
+        VarList, {Modifier, OriginalSharingModifier}, StartLoc, LParenLoc,
+        ModifierLoc, ColonLoc, EndLoc, ReductionIdScopeSpec, ReductionId,
+        UnresolvedReductions);
   }
 
   /// Build a new OpenMP 'task_reduction' clause.
@@ -10946,8 +10948,8 @@ TreeTransform<Derived>::TransformOMPReductionClause(OMPReductionClause *C) {
       UnresolvedReductions.push_back(nullptr);
   }
   return getDerived().RebuildOMPReductionClause(
-      Vars, C->getModifier(), C->getBeginLoc(), C->getLParenLoc(),
-      C->getModifierLoc(), C->getColonLoc(), C->getEndLoc(),
+      Vars, C->getModifier(), C->getOriginalSharingModifier(), C->getBeginLoc(),
+      C->getLParenLoc(), C->getModifierLoc(), C->getColonLoc(), C->getEndLoc(),
       ReductionIdScopeSpec, NameInfo, UnresolvedReductions);
 }
 

clang/lib/Serialization/ASTWriter.cpp

@@ -8020,8 +8020,9 @@ void OMPClauseWriter::VisitOMPReductionClause(OMPReductionClause *C) {
     for (auto *E : C->copy_array_elems())
       Record.AddStmt(E);
   }
-  ArrayRef<bool> PrivateFlags = C->getPrivateVariableReductionFlags();
-  Record.push_back(PrivateFlags.size());
+  auto PrivateFlags = C->private_var_reduction_flags();
+  Record.push_back(std::distance(PrivateFlags.begin(), PrivateFlags.end()));
+  // Record.push_back(PrivateFlags.size());
   for (bool Flag : PrivateFlags)
     Record.push_back(Flag);
 }