diff options
| -rw-r--r-- | clang/include/clang/Sema/SemaOpenACC.h | 83 | ||||
| -rw-r--r-- | clang/lib/Sema/SemaOpenACC.cpp | 467 | ||||
| -rw-r--r-- | clang/test/SemaOpenACC/combined-construct.cpp | 20 | ||||
| -rw-r--r-- | clang/test/SemaOpenACC/loop-construct.cpp | 39 |
4 files changed, 392 insertions, 217 deletions
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 @@ private: /// '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'. - void check(); + 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}}) {} - const ValueDecl *checkInit(); - void checkCond(); - void checkInc(const ValueDecl *Init); + void check(); }; /// 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 @@ void SemaOpenACC::ForStmtBeginHelper(SourceLocation ForLoc, 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 @@ void SemaOpenACC::ForStmtBeginHelper(SourceLocation ForLoc, } 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 @@ bool isValidLoopVariableType(QualType LoopVarTy) { 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,116 +1058,110 @@ bool isValidLoopVariableType(QualType LoopVarTy) { 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::check() { - if (SemaRef.LoopWithoutSeqInfo.Kind == OpenACCDirectiveKind::Invalid) +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; - if (AlreadyChecked) - return; - AlreadyChecked = true; + const DeclStmt *UninstRangeStmt = + IsInstantiation ? RFI.Uninstantiated->getBeginStmt() : nullptr; + const DeclStmt *RangeStmt = RFI.CurrentVersion->getBeginStmt(); - // OpenACC3.3 2.1: - // A loop associated with a loop construct that does not have a seq clause - // must be written to meet all the following conditions: - // - The loop variable must be of integer, C/C++ pointer, or C++ random-access - // iterator type. - // - The loop variable must monotonically increase or decrease in the - // direction of its termination condition. - // - The loop trip count must be computable in constant time when entering the - // loop construct. - // - // For a C++ range-based for loop, the loop variable - // identified by the above conditions is the internal iterator, such as a - // 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 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(); - // In some dependent contexts, the autogenerated range statement doesn't get - // included until instantiation, so skip for now. - if (!RangeStmt) + 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; } - 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) { +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 nullptr; + return true; } - - auto DiagLoopVar = [&]() { - if (InitChanged) { - SemaRef.Diag(Init->getBeginLoc(), diag::err_acc_loop_variable) + 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 nullptr; + return true; }; - if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(Init)) - Init = ExprTemp->getSubExpr(); - if (const auto *E = dyn_cast<Expr>(Init)) - Init = E->IgnoreParenImpCasts(); + if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(InitStmt)) + InitStmt = ExprTemp->getSubExpr(); + if (const auto *E = dyn_cast<Expr>(InitStmt)) + InitStmt = E->IgnoreParenImpCasts(); - const ValueDecl *InitVar = nullptr; - - if (const auto *BO = dyn_cast<BinaryOperator>(Init)) { + 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>(Init)) { + } 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(); @@ -1177,13 +1171,12 @@ const ValueDecl *SemaOpenACC::ForStmtBeginChecker::checkInit() { !cast<VarDecl>(InitVar)->hasInit()) return DiagLoopVar(); } - } else if (auto *CE = dyn_cast<CXXOperatorCallExpr>(Init)) { + } 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)) { @@ -1192,6 +1185,7 @@ const ValueDecl *SemaOpenACC::ForStmtBeginChecker::checkInit() { } } + // If after all of that, we haven't found a variable, give up. if (!InitVar) return DiagLoopVar(); @@ -1200,82 +1194,142 @@ const ValueDecl *SemaOpenACC::ForStmtBeginChecker::checkInit() { // Since we have one, all we need to do is ensure it is the right type. if (!isValidLoopVariableType(VarType)) { - if (InitChanged) { + 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 nullptr; + return true; } - 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. + return false; } -void SemaOpenACC::ForStmtBeginChecker::checkInc(const ValueDecl *Init) { +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; + } - 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; + return true; } - 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; + 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>(*Inc)) - Inc = ExprTemp->getSubExpr(); - if (const auto *E = dyn_cast<Expr>(*Inc)) - Inc = E->IgnoreParenImpCasts(); + if (const auto *ExprTemp = dyn_cast<ExprWithCleanups>(CondStmt)) + CondStmt = ExprTemp->getSubExpr(); + if (const auto *E = dyn_cast<Expr>(CondStmt)) + CondStmt = 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(); + 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; + } - if (!E) - return nullptr; - if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) - return dyn_cast<ValueDecl>(DRE->getDecl()); + // 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(); - if (const auto *ME = dyn_cast<MemberExpr>(E)) - if (isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts())) - return ME->getMemberDecl(); + return false; +} - return nullptr; +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; }; - const ValueDecl *IncVar = nullptr; + 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>(*Inc)) { + 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>(*Inc)) { + } else if (const auto *BO = dyn_cast<BinaryOperator>(IncStmt)) { switch (BO->getOpcode()) { default: return DiagIncVar(); @@ -1292,7 +1346,7 @@ void SemaOpenACC::ForStmtBeginChecker::checkInc(const ValueDecl *Init) { break; } IncVar = getDeclFromExpr(BO->getLHS()); - } else if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(*Inc)) { + } else if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(IncStmt)) { switch (CE->getOperator()) { default: return DiagIncVar(); @@ -1306,14 +1360,14 @@ void SemaOpenACC::ForStmtBeginChecker::checkInc(const ValueDecl *Init) { // += -= *= /= 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. + // 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)) { + } 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. @@ -1325,10 +1379,106 @@ void SemaOpenACC::ForStmtBeginChecker::checkInc(const ValueDecl *Init) { // 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()) + if (InitVar && IncVar->getCanonicalDecl() != InitVar->getCanonicalDecl()) return DiagIncVar(); - return; + 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; + + // OpenACC3.3 2.1: + // A loop associated with a loop construct that does not have a seq clause + // must be written to meet all the following conditions: + // - The loop variable must be of integer, C/C++ pointer, or C++ random-access + // iterator type. + // - The loop variable must monotonically increase or decrease in the + // direction of its termination condition. + // - The loop trip count must be computable in constant time when entering the + // loop construct. + // + // For a C++ range-based for loop, the loop variable + // identified by the above conditions is the internal iterator, such as a + // pointer, that the compiler generates to iterate the range. it is not the + // variable declared by the for loop. + + if (std::holds_alternative<RangeForInfo>(Info)) + return checkRangeFor(); + + return checkFor(); } void SemaOpenACC::ActOnForStmtBegin(SourceLocation ForLoc, const Stmt *OldFirst, @@ -1338,41 +1488,10 @@ void SemaOpenACC::ActOnForStmtBegin(SourceLocation ForLoc, const Stmt *OldFirst, 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 @@ void SemaOpenACC::ActOnForStmtBegin(SourceLocation ForLoc, const Stmt *First, 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 @@ void SemaOpenACC::ActOnRangeForStmtBegin(SourceLocation ForLoc, 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 @@ void SemaOpenACC::ActOnRangeForStmtBegin(SourceLocation ForLoc, 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { 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 @@ void LoopRules() { #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() { |