[OpenACC]Reimplement 'for' loop checks for a loop construct
The 'loop' construct has some pretty strict checks as to what the for
loop associated with it has to look like. The previous implementation
was a little convoluted and missed implementing the 'condition'
checking. Additionally, it did a bad job double-diagnosing with
templates.
This patch rewrites it in a way that does a much better job with the
double-diagnosing, and proeprly implements some checks for the
'condition' of the for loop.
diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h
index 2c9134b..47947d9 100644
--- a/clang/include/clang/Sema/SemaOpenACC.h
+++ b/clang/include/clang/Sema/SemaOpenACC.h
@@ -125,38 +125,73 @@
/// 'loop' clause enforcement, where this is 'blocked' by a compute construct.
llvm::SmallVector<OpenACCReductionClause *> ActiveReductionClauses;
- // Type to check the info about the 'for stmt'.
- struct ForStmtBeginChecker {
+ // Type to check the 'for' (or range-for) statement for compatibility with the
+ // 'loop' directive.
+ class ForStmtBeginChecker {
SemaOpenACC &SemaRef;
SourceLocation ForLoc;
- bool IsRangeFor = false;
- std::optional<const CXXForRangeStmt *> RangeFor = nullptr;
- const Stmt *Init = nullptr;
- bool InitChanged = false;
- std::optional<const Stmt *> Cond = nullptr;
- std::optional<const Stmt *> Inc = nullptr;
+ bool IsInstantiation = false;
+
+ struct RangeForInfo {
+ const CXXForRangeStmt *Uninstantiated = nullptr;
+ const CXXForRangeStmt *CurrentVersion = nullptr;
+ };
+
+ struct ForInfo {
+ const Stmt *Init = nullptr;
+ const Stmt *Condition = nullptr;
+ const Stmt *Increment = nullptr;
+ };
+
+ struct CheckForInfo {
+ ForInfo Uninst;
+ ForInfo Current;
+ };
+
+ std::variant<RangeForInfo, CheckForInfo> Info;
// Prevent us from checking 2x, which can happen with collapse & tile.
bool AlreadyChecked = false;
- ForStmtBeginChecker(SemaOpenACC &SemaRef, SourceLocation ForLoc,
- std::optional<const CXXForRangeStmt *> S)
- : SemaRef(SemaRef), ForLoc(ForLoc), IsRangeFor(true), RangeFor(S) {}
+ void checkRangeFor();
+ bool checkForInit(const Stmt *InitStmt, const ValueDecl *&InitVar,
+ bool Diag);
+ bool checkForCond(const Stmt *CondStmt, const ValueDecl *InitVar,
+ bool Diag);
+ bool checkForInc(const Stmt *IncStmt, const ValueDecl *InitVar, bool Diag);
+
+ void checkFor();
+
+ // void checkRangeFor(); ?? ERICH
+ // const ValueDecl *checkInit();
+ // void checkCond(const ValueDecl *Init);
+ // void checkInc(const ValueDecl *Init);
+ public:
+ // Checking for non-instantiation version of a Range-for.
ForStmtBeginChecker(SemaOpenACC &SemaRef, SourceLocation ForLoc,
- const Stmt *I, bool InitChanged,
- std::optional<const Stmt *> C,
- std::optional<const Stmt *> Inc)
- : SemaRef(SemaRef), ForLoc(ForLoc), IsRangeFor(false), Init(I),
- InitChanged(InitChanged), Cond(C), Inc(Inc) {}
- // Do the checking for the For/Range-For. Currently this implements the 'not
- // seq' restrictions only, and should be called either if we know we are a
- // top-level 'for' (the one associated via associated-stmt), or extended via
- // 'collapse'.
+ const CXXForRangeStmt *RangeFor)
+ : SemaRef(SemaRef), ForLoc(ForLoc), IsInstantiation(false),
+ Info(RangeForInfo{nullptr, RangeFor}) {}
+ // Checking for an instantiation of the range-for.
+ ForStmtBeginChecker(SemaOpenACC &SemaRef, SourceLocation ForLoc,
+ const CXXForRangeStmt *OldRangeFor,
+ const CXXForRangeStmt *RangeFor)
+ : SemaRef(SemaRef), ForLoc(ForLoc), IsInstantiation(true),
+ Info(RangeForInfo{OldRangeFor, RangeFor}) {}
+ // Checking for a non-instantiation version of a traditional for.
+ ForStmtBeginChecker(SemaOpenACC &SemaRef, SourceLocation ForLoc,
+ const Stmt *Init, const Stmt *Cond, const Stmt *Inc)
+ : SemaRef(SemaRef), ForLoc(ForLoc), IsInstantiation(false),
+ Info(CheckForInfo{{}, {Init, Cond, Inc}}) {}
+ // Checking for an instantiation version of a traditional for.
+ ForStmtBeginChecker(SemaOpenACC &SemaRef, SourceLocation ForLoc,
+ const Stmt *OldInit, const Stmt *OldCond,
+ const Stmt *OldInc, const Stmt *Init, const Stmt *Cond,
+ const Stmt *Inc)
+ : SemaRef(SemaRef), ForLoc(ForLoc), IsInstantiation(true),
+ Info(CheckForInfo{{OldInit, OldCond, OldInc}, {Init, Cond, Inc}}) {}
+
void check();
-
- const ValueDecl *checkInit();
- void checkCond();
- void checkInc(const ValueDecl *Init);
};
/// Helper function for checking the 'for' and 'range for' stmts.
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index 74eee9b..fc191bd 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -944,6 +944,7 @@
LoopInfo.TopLevelLoopSeen = true;
if (CollapseInfo.CurCollapseCount && *CollapseInfo.CurCollapseCount > 0) {
+ // Check the format of this loop if it is affected by the collapse.
C.check();
// OpenACC 3.3 2.9.1:
@@ -969,6 +970,7 @@
}
if (TileInfo.CurTileCount && *TileInfo.CurTileCount > 0) {
+ // Check the format of this loop if it is affected by the tile.
C.check();
if (LoopInfo.CurLevelHasLoopAlready) {
@@ -1044,13 +1046,11 @@
if (IsRandomAccessIteratorTag(ItrCategoryDecl))
return true;
- // We can also support types inherited from the
+ // We can also support tag-types inherited from the
// random_access_iterator_tag.
- for (CXXBaseSpecifier BS : ItrCategoryDecl->bases()) {
-
+ for (CXXBaseSpecifier BS : ItrCategoryDecl->bases())
if (IsRandomAccessIteratorTag(BS.getType()->getAsCXXRecordDecl()))
return true;
- }
return false;
}
@@ -1058,13 +1058,404 @@
return false;
}
+const ValueDecl *getDeclFromExpr(const Expr *E) {
+ E = E->IgnoreParenImpCasts();
+ if (const auto *FE = dyn_cast<FullExpr>(E))
+ E = FE->getSubExpr();
+ E = E->IgnoreParenImpCasts();
+
+ if (!E)
+ return nullptr;
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
+ return dyn_cast<ValueDecl>(DRE->getDecl());
+
+ if (const auto *ME = dyn_cast<MemberExpr>(E))
+ if (isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts()))
+ return ME->getMemberDecl();
+
+ return nullptr;
+}
} // namespace
+void SemaOpenACC::ForStmtBeginChecker::checkRangeFor() {
+ const RangeForInfo &RFI = std::get<RangeForInfo>(Info);
+ // If this hasn't changed since last instantiated we're done.
+ if (RFI.Uninstantiated == RFI.CurrentVersion)
+ return;
+
+ const DeclStmt *UninstRangeStmt =
+ IsInstantiation ? RFI.Uninstantiated->getBeginStmt() : nullptr;
+ const DeclStmt *RangeStmt = RFI.CurrentVersion->getBeginStmt();
+
+ // If this isn't the first time we've checked this loop, suppress any cases
+ // where we previously diagnosed.
+ if (UninstRangeStmt) {
+ const ValueDecl *InitVar =
+ cast<ValueDecl>(UninstRangeStmt->getSingleDecl());
+ QualType VarType = InitVar->getType().getNonReferenceType();
+
+ if (!isValidLoopVariableType(VarType))
+ return;
+ }
+
+ // In some dependent contexts, the autogenerated range statement doesn't get
+ // included until instantiation, so skip for now.
+ if (RangeStmt) {
+ const ValueDecl *InitVar = cast<ValueDecl>(RangeStmt->getSingleDecl());
+ QualType VarType = InitVar->getType().getNonReferenceType();
+
+ if (!isValidLoopVariableType(VarType)) {
+ SemaRef.Diag(InitVar->getBeginLoc(), diag::err_acc_loop_variable_type)
+ << SemaRef.LoopWithoutSeqInfo.Kind << VarType;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ return;
+ }
+ }
+}
+bool SemaOpenACC::ForStmtBeginChecker::checkForInit(const Stmt *InitStmt,
+ const ValueDecl *&InitVar,
+ bool Diag) {
+ // Init statement is required.
+ if (!InitStmt) {
+ if (Diag) {
+ SemaRef.Diag(ForLoc, diag::err_acc_loop_variable)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ }
+ return true;
+ }
+ auto DiagLoopVar = [this, Diag, InitStmt]() {
+ if (Diag) {
+ SemaRef.Diag(InitStmt->getBeginLoc(), diag::err_acc_loop_variable)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ }
+ return true;
+ };
+
+ if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(InitStmt))
+ InitStmt = ExprTemp->getSubExpr();
+ if (const auto *E = dyn_cast<Expr>(InitStmt))
+ InitStmt = E->IgnoreParenImpCasts();
+
+ InitVar = nullptr;
+ if (const auto *BO = dyn_cast<BinaryOperator>(InitStmt)) {
+ // Allow assignment operator here.
+
+ if (!BO->isAssignmentOp())
+ return DiagLoopVar();
+
+ const Expr *LHS = BO->getLHS()->IgnoreParenImpCasts();
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(LHS))
+ InitVar = DRE->getDecl();
+ } else if (const auto *DS = dyn_cast<DeclStmt>(InitStmt)) {
+ // Allow T t = <whatever>
+ if (!DS->isSingleDecl())
+ return DiagLoopVar();
+ InitVar = dyn_cast<ValueDecl>(DS->getSingleDecl());
+
+ // Ensure we have an initializer, unless this is a record/dependent type.
+ if (InitVar) {
+ if (!isa<VarDecl>(InitVar))
+ return DiagLoopVar();
+
+ if (!InitVar->getType()->isRecordType() &&
+ !InitVar->getType()->isDependentType() &&
+ !cast<VarDecl>(InitVar)->hasInit())
+ return DiagLoopVar();
+ }
+ } else if (auto *CE = dyn_cast<CXXOperatorCallExpr>(InitStmt)) {
+ // Allow assignment operator call.
+ if (CE->getOperator() != OO_Equal)
+ return DiagLoopVar();
+
+ const Expr *LHS = CE->getArg(0)->IgnoreParenImpCasts();
+ if (auto *DRE = dyn_cast<DeclRefExpr>(LHS)) {
+ InitVar = DRE->getDecl();
+ } else if (auto *ME = dyn_cast<MemberExpr>(LHS)) {
+ if (isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts()))
+ InitVar = ME->getMemberDecl();
+ }
+ }
+
+ // If after all of that, we haven't found a variable, give up.
+ if (!InitVar)
+ return DiagLoopVar();
+
+ InitVar = cast<ValueDecl>(InitVar->getCanonicalDecl());
+ QualType VarType = InitVar->getType().getNonReferenceType();
+
+ // Since we have one, all we need to do is ensure it is the right type.
+ if (!isValidLoopVariableType(VarType)) {
+ if (Diag) {
+ SemaRef.Diag(InitVar->getBeginLoc(), diag::err_acc_loop_variable_type)
+ << SemaRef.LoopWithoutSeqInfo.Kind << VarType;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ }
+ return true;
+ }
+
+ return false;
+}
+
+bool SemaOpenACC::ForStmtBeginChecker::checkForCond(const Stmt *CondStmt,
+ const ValueDecl *InitVar,
+ bool Diag) {
+ // A condition statement is required.
+ if (!CondStmt) {
+ if (Diag) {
+ SemaRef.Diag(ForLoc, diag::err_acc_loop_terminating_condition)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ }
+
+ return true;
+ }
+ auto DiagCondVar = [this, Diag, CondStmt] {
+ if (Diag) {
+ SemaRef.Diag(CondStmt->getBeginLoc(),
+ diag::err_acc_loop_terminating_condition)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ }
+ return true;
+ };
+
+ if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(CondStmt))
+ CondStmt = ExprTemp->getSubExpr();
+ if (const auto *E = dyn_cast<Expr>(CondStmt))
+ CondStmt = E->IgnoreParenImpCasts();
+
+ const ValueDecl *CondVar = nullptr;
+ if (const auto *BO = dyn_cast<BinaryOperator>(CondStmt)) {
+ switch (BO->getOpcode()) {
+ default:
+ return DiagCondVar();
+ case BO_EQ:
+ case BO_LT:
+ case BO_GT:
+ case BO_NE:
+ case BO_LE:
+ case BO_GE:
+ break;
+ }
+
+ // Assign the condition-var to the LHS. If it either comes back null, or
+ // the LHS doesn't match the InitVar, assign it to the RHS so that 5 < N is
+ // allowed.
+ CondVar = getDeclFromExpr(BO->getLHS());
+ if (!CondVar ||
+ (InitVar && CondVar->getCanonicalDecl() != InitVar->getCanonicalDecl()))
+ CondVar = getDeclFromExpr(BO->getRHS());
+
+ } else if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(CondStmt)) {
+ // Any of the comparison ops should be ok here, but we don't know how to
+ // handle spaceship, so disallow for now.
+ if (!CE->isComparisonOp() || CE->getOperator() == OO_Spaceship)
+ return DiagCondVar();
+
+ // Same logic here: Assign it to the LHS, unless the LHS comes back null or
+ // not equal to the init var.
+ CondVar = getDeclFromExpr(CE->getArg(0));
+ if (!CondVar ||
+ (InitVar &&
+ CondVar->getCanonicalDecl() != InitVar->getCanonicalDecl() &&
+ CE->getNumArgs() > 1))
+ CondVar = getDeclFromExpr(CE->getArg(1));
+ } else if (const auto *ME = dyn_cast<CXXMemberCallExpr>(CondStmt)) {
+ // Here there isn't much we can do besides hope it is the right variable.
+ // Codegen might have to just give up on figuring out trip count in this
+ // case?
+ CondVar = getDeclFromExpr(ME->getImplicitObjectArgument());
+ }
+
+ if (!CondVar)
+ return DiagCondVar();
+
+ // Don't consider this an error unless the init variable was properly set,
+ // else check to make sure they are the same variable.
+ if (InitVar && CondVar->getCanonicalDecl() != InitVar->getCanonicalDecl())
+ return DiagCondVar();
+
+ return false;
+}
+
+bool SemaOpenACC::ForStmtBeginChecker::checkForInc(const Stmt *IncStmt,
+ const ValueDecl *InitVar,
+ bool Diag) {
+ if (!IncStmt) {
+ if (Diag) {
+ SemaRef.Diag(ForLoc, diag::err_acc_loop_not_monotonic)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ }
+ return true;
+ }
+ auto DiagIncVar = [this, Diag, IncStmt] {
+ if (Diag) {
+ SemaRef.Diag(IncStmt->getBeginLoc(), diag::err_acc_loop_not_monotonic)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
+ diag::note_acc_construct_here)
+ << SemaRef.LoopWithoutSeqInfo.Kind;
+ }
+ return true;
+ };
+
+ if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(IncStmt))
+ IncStmt = ExprTemp->getSubExpr();
+ if (const auto *E = dyn_cast<Expr>(IncStmt))
+ IncStmt = E->IgnoreParenImpCasts();
+
+ const ValueDecl *IncVar = nullptr;
+ // Here we enforce the monotonically increase/decrease:
+ if (const auto *UO = dyn_cast<UnaryOperator>(IncStmt)) {
+ // Allow increment/decrement ops.
+ if (!UO->isIncrementDecrementOp())
+ return DiagIncVar();
+ IncVar = getDeclFromExpr(UO->getSubExpr());
+ } else if (const auto *BO = dyn_cast<BinaryOperator>(IncStmt)) {
+ switch (BO->getOpcode()) {
+ default:
+ return DiagIncVar();
+ case BO_AddAssign:
+ case BO_SubAssign:
+ case BO_MulAssign:
+ case BO_DivAssign:
+ case BO_Assign:
+ // += -= *= /= should all be fine here, this should be all of the
+ // 'monotonical' compound-assign ops.
+ // Assignment we just give up on, we could do better, and ensure that it
+ // is a binary/operator expr doing more work, but that seems like a lot
+ // of work for an error prone check.
+ break;
+ }
+ IncVar = getDeclFromExpr(BO->getLHS());
+ } else if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(IncStmt)) {
+ switch (CE->getOperator()) {
+ default:
+ return DiagIncVar();
+ case OO_PlusPlus:
+ case OO_MinusMinus:
+ case OO_PlusEqual:
+ case OO_MinusEqual:
+ case OO_StarEqual:
+ case OO_SlashEqual:
+ case OO_Equal:
+ // += -= *= /= should all be fine here, this should be all of the
+ // 'monotonical' compound-assign ops.
+ // Assignment we just give up on, we could do better, and ensure that it
+ // is a binary/operator expr doing more work, but that seems like a
+ // lot of work for an error prone check.
+ break;
+ }
+
+ IncVar = getDeclFromExpr(CE->getArg(0));
+
+ } else if (const auto *ME = dyn_cast<CXXMemberCallExpr>(IncStmt)) {
+ IncVar = getDeclFromExpr(ME->getImplicitObjectArgument());
+ // We can't really do much for member expressions, other than hope they are
+ // doing the right thing, so give up here.
+ }
+
+ if (!IncVar)
+ return DiagIncVar();
+
+ // InitVar shouldn't be null unless there was an error, so don't diagnose if
+ // that is the case. Else we should ensure that it refers to the loop
+ // value.
+ if (InitVar && IncVar->getCanonicalDecl() != InitVar->getCanonicalDecl())
+ return DiagIncVar();
+
+ return false;
+}
+
+void SemaOpenACC::ForStmtBeginChecker::checkFor() {
+ const CheckForInfo &CFI = std::get<CheckForInfo>(Info);
+
+ if (!IsInstantiation) {
+ // If this isn't an instantiation, we can just check all of these and
+ // diagnose.
+ const ValueDecl *CurInitVar = nullptr;
+ checkForInit(CFI.Current.Init, CurInitVar, /*Diag=*/true);
+ checkForCond(CFI.Current.Condition, CurInitVar, /*Diag=*/true);
+ checkForInc(CFI.Current.Increment, CurInitVar, /*DIag=*/true);
+ } else {
+ const ValueDecl *UninstInitVar = nullptr;
+ // Checking the 'init' section first. We have to always run both versions,
+ // at minimum with the 'diag' off, so that we can ensure we get the correct
+ // instantiation var for checking by later ones.
+ bool UninstInitFailed =
+ checkForInit(CFI.Uninst.Init, UninstInitVar, /*Diag=*/false);
+
+ // VarDecls are always rebuild because they are dependent, so we can do a
+ // little work to suppress some of the double checking based on whether the
+ // type is instantiation dependent. This is imperfect, but will get us most
+ // cases suppressed. Currently this only handles the 'T t =' case.
+ auto InitChanged = [=]() {
+ if (CFI.Uninst.Init == CFI.Current.Init)
+ return false;
+
+ QualType OldVDTy;
+ QualType NewVDTy;
+
+ if (const auto *DS = dyn_cast<DeclStmt>(CFI.Uninst.Init))
+ if (const VarDecl *VD = dyn_cast_if_present<VarDecl>(
+ DS->isSingleDecl() ? DS->getSingleDecl() : nullptr))
+ OldVDTy = VD->getType();
+ if (const auto *DS = dyn_cast<DeclStmt>(CFI.Current.Init))
+ if (const VarDecl *VD = dyn_cast_if_present<VarDecl>(
+ DS->isSingleDecl() ? DS->getSingleDecl() : nullptr))
+ NewVDTy = VD->getType();
+
+ if (OldVDTy.isNull() || NewVDTy.isNull())
+ return true;
+
+ return OldVDTy->isInstantiationDependentType() !=
+ NewVDTy->isInstantiationDependentType();
+ };
+
+ // Only diagnose the new 'init' if the previous version didn't fail, AND the
+ // current init changed meaningfully.
+ bool ShouldDiagNewInit = !UninstInitFailed && InitChanged();
+ const ValueDecl *CurInitVar = nullptr;
+ checkForInit(CFI.Current.Init, CurInitVar, /*Diag=*/ShouldDiagNewInit);
+
+ // Check the condition and increment only if the previous version passed,
+ // and this changed.
+ if (CFI.Uninst.Condition != CFI.Current.Condition &&
+ !checkForCond(CFI.Uninst.Condition, UninstInitVar, /*Diag=*/false))
+ checkForCond(CFI.Current.Condition, CurInitVar, /*Diag=*/true);
+ if (CFI.Uninst.Increment != CFI.Current.Increment &&
+ !checkForInc(CFI.Uninst.Increment, UninstInitVar, /*Diag=*/false))
+ checkForInc(CFI.Current.Increment, CurInitVar, /*Diag=*/true);
+ }
+}
+
void SemaOpenACC::ForStmtBeginChecker::check() {
+ // If this isn't an active loop without a seq, immediately return, nothing to
+ // check.
if (SemaRef.LoopWithoutSeqInfo.Kind == OpenACCDirectiveKind::Invalid)
return;
+ // If we've already checked, because this is a 'top level' one (and asking
+ // again because 'tile' and 'collapse' might apply), just return, nothing to
+ // do here.
if (AlreadyChecked)
return;
AlreadyChecked = true;
@@ -1084,251 +1475,10 @@
// pointer, that the compiler generates to iterate the range. it is not the
// variable declared by the for loop.
- if (IsRangeFor) {
- // If the range-for is being instantiated and didn't change, don't
- // re-diagnose.
- if (!RangeFor.has_value())
- return;
- // For a range-for, we can assume everything is 'corect' other than the type
- // of the iterator, so check that.
- const DeclStmt *RangeStmt = (*RangeFor)->getBeginStmt();
+ if (std::holds_alternative<RangeForInfo>(Info))
+ return checkRangeFor();
- // In some dependent contexts, the autogenerated range statement doesn't get
- // included until instantiation, so skip for now.
- if (!RangeStmt)
- return;
-
- const ValueDecl *InitVar = cast<ValueDecl>(RangeStmt->getSingleDecl());
- QualType VarType = InitVar->getType().getNonReferenceType();
- if (!isValidLoopVariableType(VarType)) {
- SemaRef.Diag(InitVar->getBeginLoc(), diag::err_acc_loop_variable_type)
- << SemaRef.LoopWithoutSeqInfo.Kind << VarType;
- SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
- diag::note_acc_construct_here)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- }
- return;
- }
-
- // Else we are in normal 'ForStmt', so we can diagnose everything.
- // We only have to check cond/inc if they have changed, but 'init' needs to
- // just suppress its diagnostics if it hasn't changed.
- const ValueDecl *InitVar = checkInit();
- if (Cond.has_value())
- checkCond();
- if (Inc.has_value())
- checkInc(InitVar);
-}
-const ValueDecl *SemaOpenACC::ForStmtBeginChecker::checkInit() {
- if (!Init) {
- if (InitChanged) {
- SemaRef.Diag(ForLoc, diag::err_acc_loop_variable)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
- diag::note_acc_construct_here)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- }
- return nullptr;
- }
-
- auto DiagLoopVar = [&]() {
- if (InitChanged) {
- SemaRef.Diag(Init->getBeginLoc(), diag::err_acc_loop_variable)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
- diag::note_acc_construct_here)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- }
- return nullptr;
- };
-
- if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(Init))
- Init = ExprTemp->getSubExpr();
- if (const auto *E = dyn_cast<Expr>(Init))
- Init = E->IgnoreParenImpCasts();
-
- const ValueDecl *InitVar = nullptr;
-
- if (const auto *BO = dyn_cast<BinaryOperator>(Init)) {
- // Allow assignment operator here.
-
- if (!BO->isAssignmentOp())
- return DiagLoopVar();
-
- const Expr *LHS = BO->getLHS()->IgnoreParenImpCasts();
-
- if (const auto *DRE = dyn_cast<DeclRefExpr>(LHS))
- InitVar = DRE->getDecl();
- } else if (const auto *DS = dyn_cast<DeclStmt>(Init)) {
- // Allow T t = <whatever>
- if (!DS->isSingleDecl())
- return DiagLoopVar();
-
- InitVar = dyn_cast<ValueDecl>(DS->getSingleDecl());
-
- // Ensure we have an initializer, unless this is a record/dependent type.
-
- if (InitVar) {
- if (!isa<VarDecl>(InitVar))
- return DiagLoopVar();
-
- if (!InitVar->getType()->isRecordType() &&
- !InitVar->getType()->isDependentType() &&
- !cast<VarDecl>(InitVar)->hasInit())
- return DiagLoopVar();
- }
- } else if (auto *CE = dyn_cast<CXXOperatorCallExpr>(Init)) {
- // Allow assignment operator call.
- if (CE->getOperator() != OO_Equal)
- return DiagLoopVar();
-
- const Expr *LHS = CE->getArg(0)->IgnoreParenImpCasts();
-
- if (auto *DRE = dyn_cast<DeclRefExpr>(LHS)) {
- InitVar = DRE->getDecl();
- } else if (auto *ME = dyn_cast<MemberExpr>(LHS)) {
- if (isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts()))
- InitVar = ME->getMemberDecl();
- }
- }
-
- if (!InitVar)
- return DiagLoopVar();
-
- InitVar = cast<ValueDecl>(InitVar->getCanonicalDecl());
- QualType VarType = InitVar->getType().getNonReferenceType();
-
- // Since we have one, all we need to do is ensure it is the right type.
- if (!isValidLoopVariableType(VarType)) {
- if (InitChanged) {
- SemaRef.Diag(InitVar->getBeginLoc(), diag::err_acc_loop_variable_type)
- << SemaRef.LoopWithoutSeqInfo.Kind << VarType;
- SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc,
- diag::note_acc_construct_here)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- }
- return nullptr;
- }
-
- return InitVar;
-}
-void SemaOpenACC::ForStmtBeginChecker::checkCond() {
- if (!*Cond) {
- SemaRef.Diag(ForLoc, diag::err_acc_loop_terminating_condition)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc, diag::note_acc_construct_here)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- }
- // Nothing else to do here. we could probably do some additional work to look
- // into the termination condition, but that error-prone. For now, we don't
- // implement anything other than 'there is a termination condition', and if
- // codegen/MLIR comes up with some necessary restrictions, we can implement
- // them here.
-}
-
-void SemaOpenACC::ForStmtBeginChecker::checkInc(const ValueDecl *Init) {
-
- if (!*Inc) {
- SemaRef.Diag(ForLoc, diag::err_acc_loop_not_monotonic)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc, diag::note_acc_construct_here)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- return;
- }
- auto DiagIncVar = [this] {
- SemaRef.Diag((*Inc)->getBeginLoc(), diag::err_acc_loop_not_monotonic)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- SemaRef.Diag(SemaRef.LoopWithoutSeqInfo.Loc, diag::note_acc_construct_here)
- << SemaRef.LoopWithoutSeqInfo.Kind;
- return;
- };
-
- if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(*Inc))
- Inc = ExprTemp->getSubExpr();
- if (const auto *E = dyn_cast<Expr>(*Inc))
- Inc = E->IgnoreParenImpCasts();
-
- auto getDeclFromExpr = [](const Expr *E) -> const ValueDecl * {
- E = E->IgnoreParenImpCasts();
- if (const auto *FE = dyn_cast<FullExpr>(E))
- E = FE->getSubExpr();
-
- E = E->IgnoreParenImpCasts();
-
- if (!E)
- return nullptr;
- if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
- return dyn_cast<ValueDecl>(DRE->getDecl());
-
- if (const auto *ME = dyn_cast<MemberExpr>(E))
- if (isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts()))
- return ME->getMemberDecl();
-
- return nullptr;
- };
-
- const ValueDecl *IncVar = nullptr;
-
- // Here we enforce the monotonically increase/decrease:
- if (const auto *UO = dyn_cast<UnaryOperator>(*Inc)) {
- // Allow increment/decrement ops.
- if (!UO->isIncrementDecrementOp())
- return DiagIncVar();
- IncVar = getDeclFromExpr(UO->getSubExpr());
- } else if (const auto *BO = dyn_cast<BinaryOperator>(*Inc)) {
- switch (BO->getOpcode()) {
- default:
- return DiagIncVar();
- case BO_AddAssign:
- case BO_SubAssign:
- case BO_MulAssign:
- case BO_DivAssign:
- case BO_Assign:
- // += -= *= /= should all be fine here, this should be all of the
- // 'monotonical' compound-assign ops.
- // Assignment we just give up on, we could do better, and ensure that it
- // is a binary/operator expr doing more work, but that seems like a lot
- // of work for an error prone check.
- break;
- }
- IncVar = getDeclFromExpr(BO->getLHS());
- } else if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(*Inc)) {
- switch (CE->getOperator()) {
- default:
- return DiagIncVar();
- case OO_PlusPlus:
- case OO_MinusMinus:
- case OO_PlusEqual:
- case OO_MinusEqual:
- case OO_StarEqual:
- case OO_SlashEqual:
- case OO_Equal:
- // += -= *= /= should all be fine here, this should be all of the
- // 'monotonical' compound-assign ops.
- // Assignment we just give up on, we could do better, and ensure that it
- // is a binary/operator expr doing more work, but that seems like a lot
- // of work for an error prone check.
- break;
- }
-
- IncVar = getDeclFromExpr(CE->getArg(0));
-
- } else if (const auto *ME = dyn_cast<CXXMemberCallExpr>(*Inc)) {
- IncVar = getDeclFromExpr(ME->getImplicitObjectArgument());
- // We can't really do much for member expressions, other than hope they are
- // doing the right thing, so give up here.
- }
-
- if (!IncVar)
- return DiagIncVar();
-
- // InitVar shouldn't be null unless there was an error, so don't diagnose if
- // that is the case. Else we should ensure that it refers to the loop
- // value.
- if (Init && IncVar->getCanonicalDecl() != Init->getCanonicalDecl())
- return DiagIncVar();
-
- return;
+ return checkFor();
}
void SemaOpenACC::ActOnForStmtBegin(SourceLocation ForLoc, const Stmt *OldFirst,
@@ -1338,41 +1488,10 @@
if (!getLangOpts().OpenACC)
return;
- std::optional<const Stmt *> S;
- if (OldSecond == Second)
- S = std::nullopt;
- else
- S = Second;
- std::optional<const Stmt *> T;
- if (OldThird == Third)
- S = std::nullopt;
- else
- S = Third;
-
- bool InitChanged = false;
- if (OldFirst != First) {
- InitChanged = true;
-
- // VarDecls are always rebuild because they are dependent, so we can do a
- // little work to suppress some of the double checking based on whether the
- // type is instantiation dependent.
- QualType OldVDTy;
- QualType NewVDTy;
- if (const auto *DS = dyn_cast<DeclStmt>(OldFirst))
- if (const VarDecl *VD = dyn_cast_if_present<VarDecl>(
- DS->isSingleDecl() ? DS->getSingleDecl() : nullptr))
- OldVDTy = VD->getType();
- if (const auto *DS = dyn_cast<DeclStmt>(First))
- if (const VarDecl *VD = dyn_cast_if_present<VarDecl>(
- DS->isSingleDecl() ? DS->getSingleDecl() : nullptr))
- NewVDTy = VD->getType();
-
- if (!OldVDTy.isNull() && !NewVDTy.isNull())
- InitChanged = OldVDTy->isInstantiationDependentType() !=
- NewVDTy->isInstantiationDependentType();
- }
-
- ForStmtBeginChecker FSBC{*this, ForLoc, First, InitChanged, S, T};
+ ForStmtBeginChecker FSBC{*this, ForLoc, OldFirst, OldSecond,
+ OldThird, First, Second, Third};
+ // Check if this is the top-level 'for' for a 'loop'. Else it will be checked
+ // as a part of the helper if a tile/collapse applies.
if (!LoopInfo.TopLevelLoopSeen) {
FSBC.check();
}
@@ -1385,11 +1504,12 @@
if (!getLangOpts().OpenACC)
return;
- ForStmtBeginChecker FSBC{*this, ForLoc, First, /*InitChanged=*/true,
- Second, Third};
- if (!LoopInfo.TopLevelLoopSeen) {
+ ForStmtBeginChecker FSBC{*this, ForLoc, First, Second, Third};
+
+ // Check if this is the top-level 'for' for a 'loop'. Else it will be checked
+ // as a part of the helper if a tile/collapse applies.
+ if (!LoopInfo.TopLevelLoopSeen)
FSBC.check();
- }
ForStmtBeginHelper(ForLoc, FSBC);
}
@@ -1400,14 +1520,11 @@
if (!getLangOpts().OpenACC)
return;
- std::optional<const CXXForRangeStmt *> RF;
-
- if (OldRangeFor == RangeFor)
- RF = std::nullopt;
- else
- RF = cast<CXXForRangeStmt>(RangeFor);
-
- ForStmtBeginChecker FSBC{*this, ForLoc, RF};
+ ForStmtBeginChecker FSBC{*this, ForLoc,
+ cast_if_present<CXXForRangeStmt>(OldRangeFor),
+ cast_if_present<CXXForRangeStmt>(RangeFor)};
+ // Check if this is the top-level 'for' for a 'loop'. Else it will be checked
+ // as a part of the helper if a tile/collapse applies.
if (!LoopInfo.TopLevelLoopSeen) {
FSBC.check();
}
@@ -1419,10 +1536,14 @@
if (!getLangOpts().OpenACC)
return;
- ForStmtBeginChecker FSBC{*this, ForLoc, cast<CXXForRangeStmt>(RangeFor)};
- if (!LoopInfo.TopLevelLoopSeen) {
+ ForStmtBeginChecker FSBC = {*this, ForLoc,
+ cast_if_present<CXXForRangeStmt>(RangeFor)};
+
+ // Check if this is the top-level 'for' for a 'loop'. Else it will be checked
+ // as a part of the helper if a tile/collapse applies.
+ if (!LoopInfo.TopLevelLoopSeen)
FSBC.check();
- }
+
ForStmtBeginHelper(ForLoc, FSBC);
}
diff --git a/clang/test/SemaOpenACC/combined-construct.cpp b/clang/test/SemaOpenACC/combined-construct.cpp
index b0fd05e..e7ca0ee 100644
--- a/clang/test/SemaOpenACC/combined-construct.cpp
+++ b/clang/test/SemaOpenACC/combined-construct.cpp
@@ -176,8 +176,8 @@
for(int f;;);
#pragma acc kernels loop
- // expected-error@+6 2{{OpenACC 'kernels loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'kernels loop' construct is here}}
+ // expected-error@+6{{OpenACC 'kernels loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'kernels loop' construct is here}}
// expected-error@+4{{OpenACC 'kernels loop' construct must have a terminating condition}}
// expected-note@-4{{'kernels loop' construct is here}}
// expected-error@+2{{OpenACC 'kernels loop' variable must monotonically increase or decrease ('++', '--', or compound assignment)}}
@@ -255,8 +255,8 @@
for( i = 0;;);
#pragma acc kernels loop
- // expected-error@+6 2{{OpenACC 'kernels loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'kernels loop' construct is here}}
+ // expected-error@+6{{OpenACC 'kernels loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'kernels loop' construct is here}}
// expected-error@+4{{OpenACC 'kernels loop' construct must have a terminating condition}}
// expected-note@-4{{'kernels loop' construct is here}}
// expected-error@+2{{OpenACC 'kernels loop' variable must monotonically increase or decrease ('++', '--', or compound assignment)}}
@@ -273,8 +273,8 @@
for( int j ;;);
#pragma acc serial loop
- // expected-error@+6 2{{OpenACC 'serial loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'serial loop' construct is here}}
+ // expected-error@+6{{OpenACC 'serial loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'serial loop' construct is here}}
// expected-error@+4{{OpenACC 'serial loop' construct must have a terminating condition}}
// expected-note@-4{{'serial loop' construct is here}}
// expected-error@+2{{OpenACC 'serial loop' variable must monotonically increase or decrease ('++', '--', or compound assignment)}}
@@ -342,8 +342,8 @@
for(auto X : Array);
#pragma acc kernels loop
- // expected-error@+2 2{{loop variable of loop associated with an OpenACC 'kernels loop' construct must be of integer, pointer, or random-access-iterator type (is 'SomeIterator')}}
- // expected-note@-2 2{{'kernels loop' construct is here}}
+ // expected-error@+2{{loop variable of loop associated with an OpenACC 'kernels loop' construct must be of integer, pointer, or random-access-iterator type (is 'SomeIterator')}}
+ // expected-note@-2{{'kernels loop' construct is here}}
for(auto X : HasIteratorCollection{});
#pragma acc serial loop
@@ -355,8 +355,8 @@
RandAccessIterator f;
#pragma acc serial loop
- // expected-error@+2 2{{OpenACC 'serial loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'serial loop' construct is here}}
+ // expected-error@+2{{OpenACC 'serial loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'serial loop' construct is here}}
for(f;f != end;f++);
#pragma acc kernels loop
diff --git a/clang/test/SemaOpenACC/loop-construct.cpp b/clang/test/SemaOpenACC/loop-construct.cpp
index 5616cac..7509811 100644
--- a/clang/test/SemaOpenACC/loop-construct.cpp
+++ b/clang/test/SemaOpenACC/loop-construct.cpp
@@ -167,8 +167,8 @@
for(int f;;);
#pragma acc loop
- // expected-error@+6 2{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'loop' construct is here}}
+ // expected-error@+6{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'loop' construct is here}}
// expected-error@+4{{OpenACC 'loop' construct must have a terminating condition}}
// expected-note@-4{{'loop' construct is here}}
// expected-error@+2{{OpenACC 'loop' variable must monotonically increase or decrease ('++', '--', or compound assignment)}}
@@ -246,8 +246,8 @@
for( i = 0;;);
#pragma acc loop
- // expected-error@+6 2{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'loop' construct is here}}
+ // expected-error@+6{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'loop' construct is here}}
// expected-error@+4{{OpenACC 'loop' construct must have a terminating condition}}
// expected-note@-4{{'loop' construct is here}}
// expected-error@+2{{OpenACC 'loop' variable must monotonically increase or decrease ('++', '--', or compound assignment)}}
@@ -264,8 +264,8 @@
for( int j ;;);
#pragma acc loop
- // expected-error@+6 2{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'loop' construct is here}}
+ // expected-error@+6{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'loop' construct is here}}
// expected-error@+4{{OpenACC 'loop' construct must have a terminating condition}}
// expected-note@-4{{'loop' construct is here}}
// expected-error@+2{{OpenACC 'loop' variable must monotonically increase or decrease ('++', '--', or compound assignment)}}
@@ -333,8 +333,8 @@
for(auto X : Array);
#pragma acc loop
- // expected-error@+2 2{{loop variable of loop associated with an OpenACC 'loop' construct must be of integer, pointer, or random-access-iterator type (is 'SomeIterator')}}
- // expected-note@-2 2{{'loop' construct is here}}
+ // expected-error@+2{{loop variable of loop associated with an OpenACC 'loop' construct must be of integer, pointer, or random-access-iterator type (is 'SomeIterator')}}
+ // expected-note@-2{{'loop' construct is here}}
for(auto X : HasIteratorCollection{});
#pragma acc loop
@@ -346,8 +346,8 @@
RandAccessIterator f;
#pragma acc loop
- // expected-error@+2 2{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
- // expected-note@-2 2{{'loop' construct is here}}
+ // expected-error@+2{{OpenACC 'loop' construct must have initialization clause in canonical form ('var = init' or 'T var = init'}}
+ // expected-note@-2{{'loop' construct is here}}
for(f;f != end;f++);
#pragma acc loop
@@ -373,6 +373,25 @@
#pragma acc loop
for(f = 0;f != end;f+=1);
+
+#pragma acc loop
+ for (int i = 0; 5 >= i; ++i);
+
+ int otherI;
+ // expected-error@+3{{OpenACC 'loop' construct must have a terminating condition}}
+ // expected-note@+1{{'loop' construct is here}}
+#pragma acc loop
+ for (int i = 0; otherI != 5; ++i);
+
+ // expected-error@+3{{OpenACC 'loop' construct must have a terminating condition}}
+ // expected-note@+1{{'loop' construct is here}}
+#pragma acc loop
+ for (int i = 0; i != 5 && i < 3; ++i);
+
+ // expected-error@+3{{OpenACC 'loop' variable must monotonically increase or decrease}}
+ // expected-note@+1{{'loop' construct is here}}
+#pragma acc loop
+ for (int i = 0; i != 5; ++f);
}
void inst() {
