[OpenACC][CIR] Reduction combiner lowering for min/max (llvm#163656)

These two are lowered as if they are the expression: LHS = (LHS < RHS )
? RHS : LHS;
and
LHS = (LHS < RHS ) ? LHS : RHS;

This patch generates these expressions and ensures they are properly
emitted into IR.

Note: this is dependent on
llvm#163580
and cannot be merged until that one is (or the tests will fail).

Author: erichkeane

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -13676,6 +13676,9 @@ def err_acc_reduction_recipe_no_op
             "not have a valid operation available">;
 def note_acc_reduction_recipe_noop_field
     : Note<"while forming combiner for compound type %0">;
+def note_acc_reduction_combiner_forming
+    : Note<"while forming %select{|binary operator '%1'|conditional "
+           "operator|final assignment operator}0">;
 
 // AMDGCN builtins diagnostics
 def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">;

clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.cpp

@@ -590,15 +590,18 @@ void OpenACCRecipeBuilderBase::createReductionRecipeCombiner(
       } else {
         // else we have to handle each individual field after after a
         // get-element.
+        const CIRGenRecordLayout &layout =
+            cgf.cgm.getTypes().getCIRGenRecordLayout(rd);
         for (const auto &[field, combiner] :
              llvm::zip_equal(rd->fields(), combinerRecipes)) {
           mlir::Type fieldType = cgf.convertType(field->getType());
           auto fieldPtr = cir::PointerType::get(fieldType);
+          unsigned fieldIndex = layout.getCIRFieldNo(field);
 
           mlir::Value lhsField = builder.createGetMember(
-              loc, fieldPtr, lhsArg, field->getName(), field->getFieldIndex());
+              loc, fieldPtr, lhsArg, field->getName(), fieldIndex);
           mlir::Value rhsField = builder.createGetMember(
-              loc, fieldPtr, rhsArg, field->getName(), field->getFieldIndex());
+              loc, fieldPtr, rhsArg, field->getName(), fieldIndex);
 
           emitSingleCombiner(lhsField, rhsField, combiner);
         }

clang/lib/Sema/SemaOpenACC.cpp

@@ -2996,6 +2996,8 @@ bool SemaOpenACC::CreateReductionCombinerRecipe(
 
   case OpenACCReductionOperator::Max:
   case OpenACCReductionOperator::Min:
+    BinOp = BinaryOperatorKind::BO_LT;
+    break;
   case OpenACCReductionOperator::And:
   case OpenACCReductionOperator::Or:
     // We just want a 'NYI' error in the backend, so leave an empty combiner
@@ -3011,26 +3013,80 @@ bool SemaOpenACC::CreateReductionCombinerRecipe(
 
   assert(!VarTy->isArrayType() && "Only 1 level of array allowed");
 
+  enum class CombinerFailureKind {
+    None = 0,
+    BinOp = 1,
+    Conditional = 2,
+    Assignment = 3,
+  };
+
+  auto genCombiner = [&, this](DeclRefExpr *LHSDRE, DeclRefExpr *RHSDRE)
+      -> std::pair<ExprResult, CombinerFailureKind> {
+    ExprResult BinOpRes =
+        SemaRef.BuildBinOp(SemaRef.getCurScope(), Loc, BinOp, LHSDRE, RHSDRE,
+                           /*ForFoldExpr=*/false);
+    switch (ReductionOperator) {
+    case OpenACCReductionOperator::Addition:
+    case OpenACCReductionOperator::Multiplication:
+    case OpenACCReductionOperator::BitwiseAnd:
+    case OpenACCReductionOperator::BitwiseOr:
+    case OpenACCReductionOperator::BitwiseXOr:
+      // These 5 are simple and are being done  as compound operators, so we can
+      // immediately quit here.
+      return {BinOpRes, BinOpRes.isUsable() ? CombinerFailureKind::None
+                                            : CombinerFailureKind::BinOp};
+    case OpenACCReductionOperator::Max:
+    case OpenACCReductionOperator::Min: {
+      // These are done as:
+      // LHS = (LHS < RHS) ? LHS : RHS; and LHS = (LHS < RHS) ? RHS : LHS;
+      //
+      // The BinOpRes should have been created with the less-than, so we just
+      // have to build the conditional and assignment.
+      if (!BinOpRes.isUsable())
+        return {BinOpRes, CombinerFailureKind::BinOp};
+
+      // Create the correct conditional operator, swapping the results
+      // (true/false value) depending on min/max.
+      ExprResult CondRes;
+      if (ReductionOperator == OpenACCReductionOperator::Min)
+        CondRes = SemaRef.ActOnConditionalOp(Loc, Loc, BinOpRes.get(), LHSDRE,
+                                             RHSDRE);
+      else
+        CondRes = SemaRef.ActOnConditionalOp(Loc, Loc, BinOpRes.get(), RHSDRE,
+                                             LHSDRE);
+
+      if (!CondRes.isUsable())
+        return {CondRes, CombinerFailureKind::Conditional};
+
+      // Build assignment.
+      ExprResult Assignment = SemaRef.BuildBinOp(SemaRef.getCurScope(), Loc,
+                                                 BinaryOperatorKind::BO_Assign,
+                                                 LHSDRE, CondRes.get(),
+                                                 /*ForFoldExpr=*/false);
+      return {Assignment, Assignment.isUsable()
+                              ? CombinerFailureKind::None
+                              : CombinerFailureKind::Assignment};
+    }
+    case OpenACCReductionOperator::And:
+    case OpenACCReductionOperator::Or:
+      llvm_unreachable("And/Or not implemented, but should fail earlier");
+    case OpenACCReductionOperator::Invalid:
+      llvm_unreachable("Invalid should have been caught above");
+    }
+  };
+
   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);
+      return genCombiner(LHSDRE, RHSDRE);
     }
+    return genCombiner(LHSDRE, RHSDRE);
   };
 
   struct CombinerAttemptTy {
+    CombinerFailureKind FailKind;
     VarDecl *LHS;
     DeclRefExpr *LHSDRE;
     VarDecl *RHS;
@@ -3058,9 +3114,11 @@ bool SemaOpenACC::CreateReductionCombinerRecipe(
                             RHSDecl->getBeginLoc()},
         Ty, clang::VK_LValue, RHSDecl, nullptr, NOUR_None);
 
-    ExprResult BinOpResult = tryCombiner(LHSDRE, RHSDRE, /*IncludeTrap=*/true);
+    std::pair<ExprResult, CombinerFailureKind> BinOpResult =
+        tryCombiner(LHSDRE, RHSDRE, /*IncludeTrap=*/true);
 
-    return {LHSDecl, LHSDRE, RHSDecl, RHSDRE, BinOpResult.get()};
+    return {BinOpResult.second,     LHSDecl, LHSDRE, RHSDecl, RHSDRE,
+            BinOpResult.first.get()};
   };
 
   CombinerAttemptTy TopLevelCombinerInfo = formCombiner(VarTy);
@@ -3081,12 +3139,20 @@ bool SemaOpenACC::CreateReductionCombinerRecipe(
     }
   }
 
+  auto EmitFailureNote = [&](CombinerFailureKind CFK) {
+    if (CFK == CombinerFailureKind::BinOp)
+      return Diag(Loc, diag::note_acc_reduction_combiner_forming)
+             << CFK << BinaryOperator::getOpcodeStr(BinOp);
+    return Diag(Loc, diag::note_acc_reduction_combiner_forming) << CFK;
+  };
+
   // 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;
+    EmitFailureNote(TopLevelCombinerInfo.FailKind);
     tryCombiner(TopLevelCombinerInfo.LHSDRE, TopLevelCombinerInfo.RHSDRE,
                 /*IncludeTrap=*/false);
     return true;
@@ -3098,6 +3164,7 @@ bool SemaOpenACC::CreateReductionCombinerRecipe(
     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;
+      EmitFailureNote(FieldCombinerInfo.FailKind);
       tryCombiner(FieldCombinerInfo.LHSDRE, FieldCombinerInfo.RHSDRE,
                   /*IncludeTrap=*/false);
       return true;