[OpenACC] Fix uses of getBaseOriginalType when we really want elt type. (llvm#162880)

Lately, I've been using 'getBaseOriginalType' in ArraySectionExpr
incorrectly: it gets the base-ist of element type, when in reality, I
want a single type of indirection. This patch corrects the handful of
uses that I had for it.

Author: erichkeane

clang/include/clang/AST/Expr.h

@@ -7160,6 +7160,18 @@ class ArraySectionExpr : public Expr {
   /// Return original type of the base expression for array section.
   static QualType getBaseOriginalType(const Expr *Base);
 
+  /// Return the effective 'element' type of this array section. As the array
+  /// section itself returns a collection of elements (closer to its `getBase`
+  /// type), this is only useful for figuring out the effective type of this if
+  /// it were a normal Array subscript expr.
+  QualType getElementType() const;
+
+  /// Returns the effective 'type' of the base of this array section.  This
+  /// should be the array/pointer type that this operates on.  Just
+  /// getBase->getType isn't sufficient, since it doesn't look through existing
+  /// Array sections to figure out the actual 'base' of this.
+  QualType getBaseType() const;
+
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == ArraySectionExprClass;
   }

clang/include/clang/Sema/SemaOpenACC.h

@@ -911,6 +911,7 @@ class SemaOpenACC : public SemaBase {
   ExprResult CheckReductionVar(OpenACCDirectiveKind DirectiveKind,
                                OpenACCReductionOperator ReductionOp,
                                Expr *VarExpr);
+  bool CheckReductionVarType(Expr *VarExpr);
 
   /// Called to check the 'var' type is a variable of pointer type, necessary
   /// for 'deviceptr' and 'attach' clauses. Returns true on success.

clang/lib/AST/Expr.cpp

@@ -5290,6 +5290,33 @@ QualType ArraySectionExpr::getBaseOriginalType(const Expr *Base) {
   return OriginalTy;
 }
 
+QualType ArraySectionExpr::getElementType() const {
+  QualType BaseTy = getBase()->IgnoreParenImpCasts()->getType();
+  // We only have to look into the array section exprs, else we will get the
+  // type of the base, which should already be valid.
+  if (auto *ASE = dyn_cast<ArraySectionExpr>(getBase()->IgnoreParenImpCasts()))
+    BaseTy = ASE->getElementType();
+
+  if (BaseTy->isAnyPointerType())
+    return BaseTy->getPointeeType();
+  if (BaseTy->isArrayType())
+    return BaseTy->castAsArrayTypeUnsafe()->getElementType();
+
+  // If this isn't a pointer or array, the base is a dependent expression, so
+  // just return the BaseTy anyway.
+  assert(BaseTy->isInstantiationDependentType());
+  return BaseTy;
+}
+
+QualType ArraySectionExpr::getBaseType() const {
+  // We only have to look into the array section exprs, else we will get the
+  // type of the base, which should already be valid.
+  if (auto *ASE = dyn_cast<ArraySectionExpr>(getBase()->IgnoreParenImpCasts()))
+    return ASE->getElementType();
+
+  return getBase()->IgnoreParenImpCasts()->getType();
+}
+
 RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
                            SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
     : Expr(RecoveryExprClass, T.getNonReferenceType(),

clang/lib/CIR/CodeGen/CIRGenOpenACC.cpp

@@ -73,7 +73,7 @@ CIRGenFunction::getOpenACCDataOperandInfo(const Expr *e) {
   // Array sections are special, and we have to treat them that way.
   if (const auto *section =
           dyn_cast<ArraySectionExpr>(curVarExpr->IgnoreParenImpCasts()))
-    origType = ArraySectionExpr::getBaseOriginalType(section);
+    origType = section->getElementType();
 
   mlir::Location exprLoc = cgm.getLoc(curVarExpr->getBeginLoc());
   llvm::SmallVector<mlir::Value> bounds;
@@ -84,16 +84,10 @@ CIRGenFunction::getOpenACCDataOperandInfo(const Expr *e) {
   e->printPretty(os, nullptr, getContext().getPrintingPolicy());
 
   auto addBoundType = [&](const Expr *e) {
-    if (const auto *section = dyn_cast<ArraySectionExpr>(curVarExpr)) {
-      QualType baseTy = ArraySectionExpr::getBaseOriginalType(
-          section->getBase()->IgnoreParenImpCasts());
-      if (auto *at = getContext().getAsArrayType(baseTy))
-        boundTypes.push_back(at->getElementType());
-      else
-        boundTypes.push_back(baseTy->getPointeeType());
-    } else {
+    if (const auto *section = dyn_cast<ArraySectionExpr>(curVarExpr))
+      boundTypes.push_back(section->getElementType());
+    else
       boundTypes.push_back(curVarExpr->getType());
-    }
   };
 
   addBoundType(curVarExpr);
@@ -113,8 +107,7 @@ CIRGenFunction::getOpenACCDataOperandInfo(const Expr *e) {
       if (const Expr *len = section->getLength()) {
         extent = emitOpenACCIntExpr(len);
       } else {
-        QualType baseTy = ArraySectionExpr::getBaseOriginalType(
-            section->getBase()->IgnoreParenImpCasts());
+        QualType baseTy = section->getBaseType();
         // We know this is the case as implicit lengths are only allowed for
         // array types with a constant size, or a dependent size.  AND since
         // we are codegen we know we're not dependent.

clang/lib/Sema/SemaOpenACC.cpp

@@ -2759,7 +2759,7 @@ OpenACCPrivateRecipe SemaOpenACC::CreatePrivateInitRecipe(const Expr *VarExpr) {
   // Array sections are special, and we have to treat them that way.
   if (const auto *ASE =
           dyn_cast<ArraySectionExpr>(VarExpr->IgnoreParenImpCasts()))
-    VarTy = ArraySectionExpr::getBaseOriginalType(ASE);
+    VarTy = ASE->getElementType();
 
   VarDecl *AllocaDecl = CreateAllocaDecl(
       getASTContext(), SemaRef.getCurContext(), VarExpr->getBeginLoc(),
@@ -2795,7 +2795,7 @@ SemaOpenACC::CreateFirstPrivateInitRecipe(const Expr *VarExpr) {
   // Array sections are special, and we have to treat them that way.
   if (const auto *ASE =
           dyn_cast<ArraySectionExpr>(VarExpr->IgnoreParenImpCasts()))
-    VarTy = ArraySectionExpr::getBaseOriginalType(ASE);
+    VarTy = ASE->getElementType();
 
   VarDecl *AllocaDecl = CreateAllocaDecl(
       getASTContext(), SemaRef.getCurContext(), VarExpr->getBeginLoc(),
@@ -2896,7 +2896,7 @@ OpenACCReductionRecipe SemaOpenACC::CreateReductionInitRecipe(
   // Array sections are special, and we have to treat them that way.
   if (const auto *ASE =
           dyn_cast<ArraySectionExpr>(VarExpr->IgnoreParenImpCasts()))
-    VarTy = ArraySectionExpr::getBaseOriginalType(ASE);
+    VarTy = ASE->getElementType();
 
   VarDecl *AllocaDecl = CreateAllocaDecl(
       getASTContext(), SemaRef.getCurContext(), VarExpr->getBeginLoc(),

clang/lib/Sema/SemaOpenACCClause.cpp

@@ -1915,51 +1915,34 @@ SemaOpenACC::ActOnClause(ArrayRef<const OpenACCClause *> ExistingClauses,
   return Result;
 }
 
-/// OpenACC 3.3 section 2.5.15:
-/// At a mininmum, the supported data types include ... the numerical data types
-/// in C, C++, and Fortran.
-///
-/// If the reduction var is a composite variable, each
-/// member of the composite variable must be a supported datatype for the
-/// reduction operation.
-ExprResult SemaOpenACC::CheckReductionVar(OpenACCDirectiveKind DirectiveKind,
-                                          OpenACCReductionOperator ReductionOp,
-                                          Expr *VarExpr) {
-  // For now, we only support 'scalar' types, or composites/arrays of scalar
-  // types.
-  VarExpr = VarExpr->IgnoreParenCasts();
+bool SemaOpenACC::CheckReductionVarType(Expr *VarExpr) {
   SourceLocation VarLoc = VarExpr->getBeginLoc();
 
   SmallVector<PartialDiagnosticAt> Notes;
-  QualType CurType = VarExpr->getType();
-
-  // For array like things, the expression can either be an array element
-  // (subscript expr), array section, or array type. Peel those off, and add
-  // notes in case we find an illegal kind.  We'll allow scalar or composite of
-  // scalars inside of this.
-  if (auto *ASE = dyn_cast<ArraySectionExpr>(VarExpr)) {
-    QualType BaseType = ArraySectionExpr::getBaseOriginalType(ASE);
+  // The standard isn't clear how many levels of 'array element' or 'subarray'
+  // are permitted, but we can handle as many as we need, so we'll strip them
+  // off here. This will result in CurType being the actual 'type' of the
+  // expression, which is what we are looking to check.
+  QualType CurType = isa<ArraySectionExpr>(VarExpr)
+                         ? ArraySectionExpr::getBaseOriginalType(VarExpr)
+                         : VarExpr->getType();
+
+  // This can happen when we have a dependent type in an array element that the
+  // above function has tried to 'unwrap'. Since this can only happen with
+  // dependence, just let it go.
+  if (CurType.isNull())
+    return false;
 
-    PartialDiagnostic PD = PDiag(diag::note_acc_reduction_array)
-                           << diag::OACCReductionArray::Section << BaseType;
-    Notes.push_back({ASE->getBeginLoc(), PD});
-
-    CurType = getASTContext().getBaseElementType(BaseType);
-  } else if (auto *SubExpr = dyn_cast<ArraySubscriptExpr>(VarExpr)) {
-    // Array subscript already results in the type of the thing as its type, so
-    // there is no type to change here.
-    PartialDiagnostic PD =
-        PDiag(diag::note_acc_reduction_array)
-        << diag::OACCReductionArray::Subscript
-        << SubExpr->getBase()->IgnoreParenImpCasts()->getType();
-    Notes.push_back({SubExpr->getBeginLoc(), PD});
-  } else if (auto *AT = getASTContext().getAsArrayType(CurType)) {
+  // If we are still an array type, we allow 1 level of 'unpeeling' of the
+  // array.  The standard isn't clear here whether this is allowed, but
+  // array-of-valid-things makes sense.
+  if (auto *AT = getASTContext().getAsArrayType(CurType)) {
     // If we're already the array type, peel off the array and leave the element
     // type.
-    CurType = getASTContext().getBaseElementType(AT);
     PartialDiagnostic PD = PDiag(diag::note_acc_reduction_array)
                            << diag::OACCReductionArray::ArrayTy << CurType;
     Notes.push_back({VarLoc, PD});
+    CurType = AT->getElementType();
   }
 
   auto IsValidMemberOfComposite = [](QualType Ty) {
@@ -1974,31 +1957,26 @@ ExprResult SemaOpenACC::CheckReductionVar(OpenACCDirectiveKind DirectiveKind,
     for (auto [Loc, PD] : Notes)
       Diag(Loc, PD);
 
-    Diag(VarLoc, diag::note_acc_reduction_type_summary);
+    return Diag(VarLoc, diag::note_acc_reduction_type_summary);
   };
 
   // If the type is already scalar, or is dependent, just give up.
   if (IsValidMemberOfComposite(CurType)) {
     // Nothing to do here, is valid.
   } else if (auto *RD = CurType->getAsRecordDecl()) {
-    if (!RD->isStruct() && !RD->isClass()) {
-      EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
-                            << RD << diag::OACCReductionTy::NotClassStruct);
-      return ExprError();
-    }
+    if (!RD->isStruct() && !RD->isClass())
+      return EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
+                                   << RD
+                                   << diag::OACCReductionTy::NotClassStruct);
 
-    if (!RD->isCompleteDefinition()) {
-      EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
-                            << RD << diag::OACCReductionTy::NotComplete);
-      return ExprError();
-    }
+    if (!RD->isCompleteDefinition())
+      return EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
+                                   << RD << diag::OACCReductionTy::NotComplete);
 
     if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD);
-        CXXRD && !CXXRD->isAggregate()) {
-      EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
-                            << CXXRD << diag::OACCReductionTy::NotAgg);
-      return ExprError();
-    }
+        CXXRD && !CXXRD->isAggregate())
+      return EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
+                                   << CXXRD << diag::OACCReductionTy::NotAgg);
 
     for (FieldDecl *FD : RD->fields()) {
       if (!IsValidMemberOfComposite(FD->getType())) {
@@ -2007,17 +1985,37 @@ ExprResult SemaOpenACC::CheckReductionVar(OpenACCDirectiveKind DirectiveKind,
             << FD->getName() << RD->getName();
         Notes.push_back({FD->getBeginLoc(), PD});
         // TODO: member here.note_acc_reduction_member_of_composite
-        EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
-                              << FD->getType()
-                              << diag::OACCReductionTy::MemberNotScalar);
-        return ExprError();
+        return EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
+                                     << FD->getType()
+                                     << diag::OACCReductionTy::MemberNotScalar);
       }
     }
   } else {
-    EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
-                          << CurType << diag::OACCReductionTy::NotScalar);
+    return EmitDiags(VarLoc, PDiag(diag::err_acc_reduction_type)
+                                 << CurType
+                                 << diag::OACCReductionTy::NotScalar);
   }
 
+  return false;
+}
+
+/// OpenACC 3.3 section 2.5.15:
+/// At a mininmum, the supported data types include ... the numerical data types
+/// in C, C++, and Fortran.
+///
+/// If the reduction var is a composite variable, each
+/// member of the composite variable must be a supported datatype for the
+/// reduction operation.
+ExprResult SemaOpenACC::CheckReductionVar(OpenACCDirectiveKind DirectiveKind,
+                                          OpenACCReductionOperator ReductionOp,
+                                          Expr *VarExpr) {
+  // For now, we only support 'scalar' types, or composites/arrays of scalar
+  // types.
+  VarExpr = VarExpr->IgnoreParenCasts();
+
+  if (CheckReductionVarType(VarExpr))
+    return ExprError();
+
   // OpenACC3.3: 2.9.11: Reduction clauses on nested constructs for the same
   // reduction 'var' must have the same reduction operator.
   if (!VarExpr->isInstantiationDependent()) {

clang/test/SemaOpenACC/combined-construct-reduction-clause.cpp

@@ -166,19 +166,17 @@ void uses(unsigned Parm) {
 
   CompositeHasComposite CoCArr[5];
   // expected-error@+4{{invalid type 'struct CompositeOfScalars' used in OpenACC 'reduction' variable reference; type is not a scalar value}}
-  // expected-note@+3{{used as element type of array type 'CompositeHasComposite'}}
+  // expected-note@+3{{used as element type of array type 'CompositeHasComposite[5]'}}
   // expected-note@#COS_FIELD{{used as field 'COS' of composite 'CompositeHasComposite'}}
   // expected-note@+1{{OpenACC 'reduction' variable reference must be a scalar variable or a composite of scalars, or an array, sub-array, or element of scalar types}}
 #pragma acc parallel loop reduction(+:CoCArr)
     for(int i = 0; i < 5; ++i);
-  // expected-error@+4{{invalid type 'struct CompositeOfScalars' used in OpenACC 'reduction' variable reference; type is not a scalar value}}
-  // expected-note@+3{{used as element type of array type 'CompositeHasComposite[5]'}}
+  // expected-error@+3{{invalid type 'struct CompositeOfScalars' used in OpenACC 'reduction' variable reference; type is not a scalar value}}
   // expected-note@#COS_FIELD{{used as field 'COS' of composite 'CompositeHasComposite'}}
   // expected-note@+1{{OpenACC 'reduction' variable reference must be a scalar variable or a composite of scalars, or an array, sub-array, or element of scalar types}}
 #pragma acc parallel loop reduction(+:CoCArr[3])
     for(int i = 0; i < 5; ++i);
-  // expected-error@+4{{invalid type 'struct CompositeOfScalars' used in OpenACC 'reduction' variable reference; type is not a scalar value}}
-  // expected-note@+3{{used as element type of sub-array type 'CompositeHasComposite'}}
+  // expected-error@+3{{invalid type 'struct CompositeOfScalars' used in OpenACC 'reduction' variable reference; type is not a scalar value}}
   // expected-note@#COS_FIELD{{used as field 'COS' of composite 'CompositeHasComposite'}}
   // expected-note@+1{{OpenACC 'reduction' variable reference must be a scalar variable or a composite of scalars, or an array, sub-array, or element of scalar types}}
 #pragma acc parallel loop reduction(+:CoCArr[1:1])

clang/test/SemaOpenACC/compute-construct-reduction-clause.c

@@ -72,8 +72,7 @@ void uses(unsigned Parm) {
   while (1);
 
   struct CompositeHasComposite ChCArray[5];
-  // expected-error@+4{{invalid type 'struct CompositeOfScalars' used in OpenACC 'reduction' variable reference; type is not a scalar value}}
-  // expected-note@+3{{used as element type of sub-array type 'struct CompositeHasComposite'}}
+  // expected-error@+3{{invalid type 'struct CompositeOfScalars' used in OpenACC 'reduction' variable reference; type is not a scalar value}}
   // expected-note@#COS_FIELD{{used as field 'COS' of composite 'CompositeHasComposite'}}
   // expected-note@+1{{OpenACC 'reduction' variable reference must be a scalar variable or a composite of scalars, or an array, sub-array, or element of scalar types}}
 #pragma acc parallel reduction(&: CoS, Array[I], ChCArray[0:I])