Reapply "[OpenACC] Sema changes for +*&|^ reduction combiner recipes (… (https://github.com/llvm/llvm-project/pull/162920)

clang/include/clang/AST/OpenACCClause.h

@@ -1277,7 +1277,7 @@ class OpenACCCreateClause final
 };
 
 // A structure to stand in for the recipe on a reduction.  RecipeDecl is the
-// 'main' declaration used for initializaiton, which is fixed. 
+// 'main' declaration used for initializaiton, which is fixed.
 struct OpenACCReductionRecipe {
   VarDecl *AllocaDecl;
 
@@ -1297,36 +1297,69 @@ struct OpenACCReductionRecipe {
   // -For a struct without the operator, this will be 1 element per field, which
   //  should be the combiner for that element.
   // -For an array of any of the above, it will be the above for the element.
-  llvm::SmallVector<CombinerRecipe, 1> CombinerRecipes;
+  // Note: These are necessarily stored in either Trailing Storage (when in the
+  // AST), or in a separate collection when being semantically analyzed.
+  llvm::ArrayRef<CombinerRecipe> CombinerRecipes;
 
   OpenACCReductionRecipe(VarDecl *A, llvm::ArrayRef<CombinerRecipe> Combiners)
       : AllocaDecl(A), CombinerRecipes(Combiners) {}
 
   bool isSet() const { return AllocaDecl; }
-  static OpenACCReductionRecipe Empty() {
-    return OpenACCReductionRecipe(/*AllocaDecl=*/nullptr, {});
+};
+
+// A version of the above that is used for semantic analysis, at a time before
+// the OpenACCReductionClause node has been created.  This one has storage for
+// the CombinerRecipe, since Trailing storage for it doesn't exist yet.
+struct OpenACCReductionRecipeWithStorage : OpenACCReductionRecipe {
+  llvm::SmallVector<CombinerRecipe, 1> CombinerRecipeStorage;
+
+  OpenACCReductionRecipeWithStorage(VarDecl *A,
+                                    llvm::ArrayRef<CombinerRecipe> Combiners)
+      : OpenACCReductionRecipe(A, {}), CombinerRecipeStorage(Combiners) {
+    CombinerRecipes = CombinerRecipeStorage;
+  }
+  static OpenACCReductionRecipeWithStorage Empty() {
+    return OpenACCReductionRecipeWithStorage(/*AllocaDecl=*/nullptr, {});
   }
 };
 
 class OpenACCReductionClause final
     : public OpenACCClauseWithVarList,
       private llvm::TrailingObjects<OpenACCReductionClause, Expr *,
-                                    OpenACCReductionRecipe> {
+                                    OpenACCReductionRecipe,
+                                    OpenACCReductionRecipe::CombinerRecipe> {
   friend TrailingObjects;
   OpenACCReductionOperator Op;
 
   OpenACCReductionClause(SourceLocation BeginLoc, SourceLocation LParenLoc,
                          OpenACCReductionOperator Operator,
                          ArrayRef<Expr *> VarList,
-                         ArrayRef<OpenACCReductionRecipe> Recipes,
+                         ArrayRef<OpenACCReductionRecipeWithStorage> Recipes,
                          SourceLocation EndLoc)
       : OpenACCClauseWithVarList(OpenACCClauseKind::Reduction, BeginLoc,
                                  LParenLoc, EndLoc),
         Op(Operator) {
-          assert(VarList.size() == Recipes.size());
+    assert(VarList.size() == Recipes.size());
     setExprs(getTrailingObjects<Expr *>(VarList.size()), VarList);
-    llvm::uninitialized_copy(Recipes, getTrailingObjects<
-                             OpenACCReductionRecipe > ());
+
+    // Since we're using trailing storage on this node to store the 'combiner'
+    // recipes of the Reduction Recipes (which have a 1:M relationship), we need
+    // to ensure we get the ArrayRef of each of our combiner 'correct'.
+    OpenACCReductionRecipe::CombinerRecipe *CurCombinerLoc =
+        getTrailingObjects<OpenACCReductionRecipe::CombinerRecipe>();
+    for (const auto &[Idx, R] : llvm::enumerate(Recipes)) {
+
+      // ArrayRef to the 'correct' data location in trailing storage.
+      llvm::MutableArrayRef<OpenACCReductionRecipe::CombinerRecipe>
+          NewCombiners{CurCombinerLoc, R.CombinerRecipes.size()};
+      CurCombinerLoc += R.CombinerRecipes.size();
+
+      llvm::uninitialized_copy(R.CombinerRecipes, NewCombiners.begin());
+
+      // Placement new into the correct location in trailng storage.
+      new (&getTrailingObjects<OpenACCReductionRecipe>()[Idx])
+          OpenACCReductionRecipe(R.AllocaDecl, NewCombiners);
+    }
   }
 
 public:
@@ -1347,13 +1380,17 @@ class OpenACCReductionClause final
   static OpenACCReductionClause *
   Create(const ASTContext &C, SourceLocation BeginLoc, SourceLocation LParenLoc,
          OpenACCReductionOperator Operator, ArrayRef<Expr *> VarList,
-         ArrayRef<OpenACCReductionRecipe> Recipes, SourceLocation EndLoc);
+         ArrayRef<OpenACCReductionRecipeWithStorage> Recipes,
+         SourceLocation EndLoc);
 
   OpenACCReductionOperator getReductionOp() const { return Op; }
 
   size_t numTrailingObjects(OverloadToken<Expr *>) const {
     return getExprs().size();
   }
+  size_t numTrailingObjects(OverloadToken<OpenACCReductionRecipe>) const {
+    return getExprs().size();
+  }
 };
 
 class OpenACCLinkClause final

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -13665,6 +13665,11 @@ def warn_acc_var_referenced_lacks_op
               "reference has no effect">,
       InGroup<DiagGroup<"openacc-var-lacks-operation">>,
       DefaultError;
+def err_acc_reduction_recipe_no_op
+    : Error<"variable of type %0 referenced in OpenACC 'reduction' clause does "
+            "not have a valid operation available">;
+def note_acc_reduction_recipe_noop_field
+    : Note<"while forming combiner for compound type %0">;
 
 // AMDGCN builtins diagnostics
 def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">;

clang/include/clang/Sema/SemaOpenACC.h

@@ -228,6 +228,11 @@ class SemaOpenACC : public SemaBase {
 
   bool DiagnoseAllowedClauses(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
                               SourceLocation ClauseLoc);
+  bool CreateReductionCombinerRecipe(
+      SourceLocation loc, OpenACCReductionOperator ReductionOperator,
+      QualType VarTy,
+      llvm::SmallVectorImpl<OpenACCReductionRecipe::CombinerRecipe>
+          &CombinerRecipes);
 
 public:
   // Needed from the visitor, so should be public.
@@ -240,7 +245,7 @@ class SemaOpenACC : public SemaBase {
 
   OpenACCPrivateRecipe CreatePrivateInitRecipe(const Expr *VarExpr);
   OpenACCFirstPrivateRecipe CreateFirstPrivateInitRecipe(const Expr *VarExpr);
-  OpenACCReductionRecipe
+  OpenACCReductionRecipeWithStorage
   CreateReductionInitRecipe(OpenACCReductionOperator ReductionOperator,
                             const Expr *VarExpr);
 
@@ -946,12 +951,14 @@ class SemaOpenACC : public SemaBase {
                   ArrayRef<Expr *> IntExprs, SourceLocation EndLoc);
   // Does the checking for a 'reduction ' clause that needs to be done in
   // dependent and not dependent cases.
-  OpenACCClause *CheckReductionClause(
-      ArrayRef<const OpenACCClause *> ExistingClauses,
-      OpenACCDirectiveKind DirectiveKind, SourceLocation BeginLoc,
-      SourceLocation LParenLoc, OpenACCReductionOperator ReductionOp,
-      ArrayRef<Expr *> Vars, ArrayRef<OpenACCReductionRecipe> Recipes,
-      SourceLocation EndLoc);
+  OpenACCClause *
+  CheckReductionClause(ArrayRef<const OpenACCClause *> ExistingClauses,
+                       OpenACCDirectiveKind DirectiveKind,
+                       SourceLocation BeginLoc, SourceLocation LParenLoc,
+                       OpenACCReductionOperator ReductionOp,
+                       ArrayRef<Expr *> Vars,
+                       ArrayRef<OpenACCReductionRecipeWithStorage> Recipes,
+                       SourceLocation EndLoc);
 
   ExprResult BuildOpenACCAsteriskSizeExpr(SourceLocation AsteriskLoc);
   ExprResult ActOnOpenACCAsteriskSizeExpr(SourceLocation AsteriskLoc);

clang/lib/AST/OpenACCClause.cpp

@@ -506,11 +506,17 @@ OpenACCDeviceTypeClause *OpenACCDeviceTypeClause::Create(
 OpenACCReductionClause *OpenACCReductionClause::Create(
     const ASTContext &C, SourceLocation BeginLoc, SourceLocation LParenLoc,
     OpenACCReductionOperator Operator, ArrayRef<Expr *> VarList,
-    ArrayRef<OpenACCReductionRecipe> Recipes,
+    ArrayRef<OpenACCReductionRecipeWithStorage> Recipes,
     SourceLocation EndLoc) {
-  void *Mem = C.Allocate(
-      OpenACCReductionClause::totalSizeToAlloc<Expr *, OpenACCReductionRecipe>(
-          VarList.size(), Recipes.size()));
+  size_t NumCombiners = llvm::accumulate(
+      Recipes, 0, [](size_t Num, const OpenACCReductionRecipe &R) {
+        return Num + R.CombinerRecipes.size();
+      });
+
+  void *Mem = C.Allocate(OpenACCReductionClause::totalSizeToAlloc<
+                         Expr *, OpenACCReductionRecipe,
+                         OpenACCReductionRecipe::CombinerRecipe>(
+      VarList.size(), Recipes.size(), NumCombiners));
   return new (Mem) OpenACCReductionClause(BeginLoc, LParenLoc, Operator,
                                           VarList, Recipes, EndLoc);
 }

clang/lib/Sema/SemaOpenACC.cpp

@@ -2883,12 +2883,12 @@ SemaOpenACC::CreateFirstPrivateInitRecipe(const Expr *VarExpr) {
   return OpenACCFirstPrivateRecipe(AllocaDecl, Temporary);
 }
 
-OpenACCReductionRecipe SemaOpenACC::CreateReductionInitRecipe(
+OpenACCReductionRecipeWithStorage SemaOpenACC::CreateReductionInitRecipe(
     OpenACCReductionOperator ReductionOperator, const Expr *VarExpr) {
   // We don't strip bounds here, so that we are doing our recipe init at the
   // 'lowest' possible level.  Codegen is going to have to do its own 'looping'.
   if (!VarExpr || VarExpr->getType()->isDependentType())
-    return OpenACCReductionRecipe::Empty();
+    return OpenACCReductionRecipeWithStorage::Empty();
 
   QualType VarTy =
       VarExpr->getType().getNonReferenceType().getUnqualifiedType();
@@ -2898,6 +2898,15 @@ OpenACCReductionRecipe SemaOpenACC::CreateReductionInitRecipe(
           dyn_cast<ArraySectionExpr>(VarExpr->IgnoreParenImpCasts()))
     VarTy = ASE->getElementType();
 
+  llvm::SmallVector<OpenACCReductionRecipe::CombinerRecipe, 1> CombinerRecipes;
+
+  // We use the 'set-ness' of the alloca-decl to determine whether the combiner
+  // is 'set' or not, so we can skip any attempts at it if we're going to fail
+  // at any of the combiners.
+  if (CreateReductionCombinerRecipe(VarExpr->getBeginLoc(), ReductionOperator,
+                                    VarTy, CombinerRecipes))
+    return OpenACCReductionRecipeWithStorage::Empty();
+
   VarDecl *AllocaDecl = CreateAllocaDecl(
       getASTContext(), SemaRef.getCurContext(), VarExpr->getBeginLoc(),
       &getASTContext().Idents.get("openacc.reduction.init"), VarTy);
@@ -2946,5 +2955,163 @@ OpenACCReductionRecipe SemaOpenACC::CreateReductionInitRecipe(
     AllocaDecl->setInit(Init.get());
     AllocaDecl->setInitStyle(VarDecl::CallInit);
   }
-  return OpenACCReductionRecipe(AllocaDecl, {});
+
+  return OpenACCReductionRecipeWithStorage(AllocaDecl, CombinerRecipes);
+}
+
+bool SemaOpenACC::CreateReductionCombinerRecipe(
+    SourceLocation Loc, OpenACCReductionOperator ReductionOperator,
+    QualType VarTy,
+    llvm::SmallVectorImpl<OpenACCReductionRecipe::CombinerRecipe>
+        &CombinerRecipes) {
+  // Now we can try to generate the 'combiner' recipe. This is a little
+  // complicated in that if the 'VarTy' is an array type, we want to take its
+  // element type so we can generate that.  Additionally, if this is a struct,
+  // we have two options: If there is overloaded operators, we want to take
+  // THOSE, else we want to do the individual elements.
+
+  BinaryOperatorKind BinOp;
+  switch (ReductionOperator) {
+  case OpenACCReductionOperator::Invalid:
+    // This can only happen when there is an error, and since these inits
+    // are used for code generation, we can just ignore/not bother doing any
+    // initialization here.
+    CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+    return false;
+  case OpenACCReductionOperator::Addition:
+    BinOp = BinaryOperatorKind::BO_AddAssign;
+    break;
+  case OpenACCReductionOperator::Multiplication:
+    BinOp = BinaryOperatorKind::BO_MulAssign;
+    break;
+  case OpenACCReductionOperator::BitwiseAnd:
+    BinOp = BinaryOperatorKind::BO_AndAssign;
+    break;
+  case OpenACCReductionOperator::BitwiseOr:
+    BinOp = BinaryOperatorKind::BO_OrAssign;
+    break;
+  case OpenACCReductionOperator::BitwiseXOr:
+    BinOp = BinaryOperatorKind::BO_XorAssign;
+    break;
+
+  case OpenACCReductionOperator::Max:
+  case OpenACCReductionOperator::Min:
+  case OpenACCReductionOperator::And:
+  case OpenACCReductionOperator::Or:
+    // We just want a 'NYI' error in the backend, so leave an empty combiner
+    // recipe, and claim success.
+    CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+    return false;
+  }
+
+  // If VarTy is an array type, at the top level only, we want to do our
+  // compares/decomp/etc at the element level.
+  if (auto *AT = getASTContext().getAsArrayType(VarTy))
+    VarTy = AT->getElementType();
+
+  assert(!VarTy->isArrayType() && "Only 1 level of array allowed");
+
+  auto tryCombiner = [&, this](DeclRefExpr *LHSDRE, DeclRefExpr *RHSDRE,
+                               bool IncludeTrap) {
+    // TODO: OpenACC: we have to figure out based on the bin-op how to do the
+    // ones that we can't just use compound operators for.  So &&, ||, max, and
+    // min aren't really clear what we could do here.
+    if (IncludeTrap) {
+      // Trap all of the errors here, we'll emit our own at the end.
+      Sema::TentativeAnalysisScope Trap{SemaRef};
+
+      return SemaRef.BuildBinOp(SemaRef.getCurScope(), Loc, BinOp, LHSDRE,
+                                RHSDRE,
+                                /*ForFoldExpr=*/false);
+    } else {
+      return SemaRef.BuildBinOp(SemaRef.getCurScope(), Loc, BinOp, LHSDRE,
+                                RHSDRE,
+                                /*ForFoldExpr=*/false);
+    }
+  };
+
+  struct CombinerAttemptTy {
+    VarDecl *LHS;
+    DeclRefExpr *LHSDRE;
+    VarDecl *RHS;
+    DeclRefExpr *RHSDRE;
+    Expr *Op;
+  };
+
+  auto formCombiner = [&, this](QualType Ty) -> CombinerAttemptTy {
+    VarDecl *LHSDecl = CreateAllocaDecl(
+        getASTContext(), SemaRef.getCurContext(), Loc,
+        &getASTContext().Idents.get("openacc.reduction.combiner.lhs"), Ty);
+    auto *LHSDRE = DeclRefExpr::Create(
+        getASTContext(), NestedNameSpecifierLoc{}, SourceLocation{}, LHSDecl,
+        /*ReferstoEnclosingVariableOrCapture=*/false,
+        DeclarationNameInfo{DeclarationName{LHSDecl->getDeclName()},
+                            LHSDecl->getBeginLoc()},
+        Ty, clang::VK_LValue, LHSDecl, nullptr, NOUR_None);
+    VarDecl *RHSDecl = CreateAllocaDecl(
+        getASTContext(), SemaRef.getCurContext(), Loc,
+        &getASTContext().Idents.get("openacc.reduction.combiner.lhs"), Ty);
+    auto *RHSDRE = DeclRefExpr::Create(
+        getASTContext(), NestedNameSpecifierLoc{}, SourceLocation{}, RHSDecl,
+        /*ReferstoEnclosingVariableOrCapture=*/false,
+        DeclarationNameInfo{DeclarationName{RHSDecl->getDeclName()},
+                            RHSDecl->getBeginLoc()},
+        Ty, clang::VK_LValue, RHSDecl, nullptr, NOUR_None);
+
+    ExprResult BinOpResult = tryCombiner(LHSDRE, RHSDRE, /*IncludeTrap=*/true);
+
+    return {LHSDecl, LHSDRE, RHSDecl, RHSDRE, BinOpResult.get()};
+  };
+
+  CombinerAttemptTy TopLevelCombinerInfo = formCombiner(VarTy);
+
+  if (TopLevelCombinerInfo.Op) {
+    if (!TopLevelCombinerInfo.Op->containsErrors() &&
+        TopLevelCombinerInfo.Op->isInstantiationDependent()) {
+      // If this is instantiation dependent, we're just going to 'give up' here
+      // and count on us to get it right during instantaition.
+      CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+      return false;
+    } else if (!TopLevelCombinerInfo.Op->containsErrors()) {
+      // Else, we succeeded, we can just return this combiner.
+      CombinerRecipes.push_back({TopLevelCombinerInfo.LHS,
+                                 TopLevelCombinerInfo.RHS,
+                                 TopLevelCombinerInfo.Op});
+      return false;
+    }
+  }
+
+  // Since the 'root' level didn't fail, the only thing that could be successful
+  // is a struct that we decompose on its individual fields.
+
+  RecordDecl *RD = VarTy->getAsRecordDecl();
+  if (!RD) {
+    Diag(Loc, diag::err_acc_reduction_recipe_no_op) << VarTy;
+    tryCombiner(TopLevelCombinerInfo.LHSDRE, TopLevelCombinerInfo.RHSDRE,
+                /*IncludeTrap=*/false);
+    return true;
+  }
+
+  for (const FieldDecl *FD : RD->fields()) {
+    CombinerAttemptTy FieldCombinerInfo = formCombiner(FD->getType());
+
+    if (!FieldCombinerInfo.Op || FieldCombinerInfo.Op->containsErrors()) {
+      Diag(Loc, diag::err_acc_reduction_recipe_no_op) << FD->getType();
+      Diag(FD->getBeginLoc(), diag::note_acc_reduction_recipe_noop_field) << RD;
+      tryCombiner(FieldCombinerInfo.LHSDRE, FieldCombinerInfo.RHSDRE,
+                  /*IncludeTrap=*/false);
+      return true;
+    }
+
+    if (FieldCombinerInfo.Op->isInstantiationDependent()) {
+      // If this is instantiation dependent, we're just going to 'give up' here
+      // and count on us to get it right during instantaition.
+      CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+    } else {
+      CombinerRecipes.push_back(
+          {FieldCombinerInfo.LHS, FieldCombinerInfo.RHS, FieldCombinerInfo.Op});
+    }
+  }
+
+  return false;
 }

clang/lib/Sema/SemaOpenACCClause.cpp

@@ -1772,7 +1772,7 @@ OpenACCClause *SemaOpenACCClauseVisitor::VisitReductionClause(
   }
 
   SmallVector<Expr *> ValidVars;
-  SmallVector<OpenACCReductionRecipe> Recipes;
+  SmallVector<OpenACCReductionRecipeWithStorage> Recipes;
 
   for (Expr *Var : Clause.getVarList()) {
     ExprResult Res = SemaRef.CheckReductionVar(Clause.getDirectiveKind(),
@@ -2196,7 +2196,7 @@ OpenACCClause *SemaOpenACC::CheckReductionClause(
     ArrayRef<const OpenACCClause *> ExistingClauses,
     OpenACCDirectiveKind DirectiveKind, SourceLocation BeginLoc,
     SourceLocation LParenLoc, OpenACCReductionOperator ReductionOp,
-    ArrayRef<Expr *> Vars, ArrayRef<OpenACCReductionRecipe> Recipes,
+    ArrayRef<Expr *> Vars, ArrayRef<OpenACCReductionRecipeWithStorage> Recipes,
     SourceLocation EndLoc) {
   if (DirectiveKind == OpenACCDirectiveKind::Loop ||
       isOpenACCCombinedDirectiveKind(DirectiveKind)) {