Author: erichkeane Date: 2025-04-30T12:02:52-07:00 New Revision: ea0e6e33835fe9fada89f77cc3a6cd74eb690067
URL: https://github.com/llvm/llvm-project/commit/ea0e6e33835fe9fada89f77cc3a6cd74eb690067 DIFF: https://github.com/llvm/llvm-project/commit/ea0e6e33835fe9fada89f77cc3a6cd74eb690067.diff LOG: [OpenACC] Improve 'loop' content restrictions The 'loop' construct has some limitations that are not particularly clear on what can be in the for-loop. This patch adds some restriction so that the increment can only be a addition or subtraction operation, BUT starts allowing Itr = Itr +/- N forms. Added: Modified: clang/lib/Sema/SemaOpenACC.cpp clang/test/SemaOpenACC/loop-construct.cpp Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp index fc191bd95a35e..4ad47c7e58bea 100644 --- a/clang/lib/Sema/SemaOpenACC.cpp +++ b/clang/lib/Sema/SemaOpenACC.cpp @@ -1275,11 +1275,8 @@ bool SemaOpenACC::ForStmtBeginChecker::checkForCond(const Stmt *CondStmt, 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()); + } else { + return DiagCondVar(); } if (!CondVar) @@ -1293,6 +1290,59 @@ bool SemaOpenACC::ForStmtBeginChecker::checkForCond(const Stmt *CondStmt, return false; } +namespace { +// Helper to check the RHS of an assignment during for's step. We can allow +// InitVar = InitVar + N, InitVar = N + InitVar, and Initvar = Initvar - N, +// where N is an integer. +bool isValidForIncRHSAssign(const ValueDecl *InitVar, const Expr *RHS) { + + auto isValid = [](const ValueDecl *InitVar, const Expr *InnerLHS, + const Expr *InnerRHS, bool IsAddition) { + // ONE of the sides has to be an integer type. + if (!InnerLHS->getType()->isIntegerType() && + !InnerRHS->getType()->isIntegerType()) + return false; + + // If the init var is already an error, don't bother trying to check for + // it. + if (!InitVar) + return true; + + const ValueDecl *LHSDecl = getDeclFromExpr(InnerLHS); + const ValueDecl *RHSDecl = getDeclFromExpr(InnerRHS); + // If we can't get a declaration, this is probably an error, so give up. + if (!LHSDecl || !RHSDecl) + return true; + + // If the LHS is the InitVar, the other must be int, so this is valid. + if (LHSDecl->getCanonicalDecl() == + InitVar->getCanonicalDecl()) + return true; + + // Subtraction doesn't allow the RHS to be init var, so this is invalid. + if (!IsAddition) + return false; + + return RHSDecl->getCanonicalDecl() == + InitVar->getCanonicalDecl(); + }; + + if (const auto *BO = dyn_cast<BinaryOperator>(RHS)) { + BinaryOperatorKind OpC = BO->getOpcode(); + if (OpC != BO_Add && OpC != BO_Sub) + return false; + return isValid(InitVar, BO->getLHS(), BO->getRHS(), OpC == BO_Add); + } else if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(RHS)) { + OverloadedOperatorKind Op = CE->getOperator(); + if (Op != OO_Plus && Op != OO_Minus) + return false; + return isValid(InitVar, CE->getArg(0), CE->getArg(1), Op == OO_Plus); + } + + return false; +} +} // namespace + bool SemaOpenACC::ForStmtBeginChecker::checkForInc(const Stmt *IncStmt, const ValueDecl *InitVar, bool Diag) { @@ -1335,14 +1385,12 @@ bool SemaOpenACC::ForStmtBeginChecker::checkForInc(const Stmt *IncStmt, return DiagIncVar(); case BO_AddAssign: case BO_SubAssign: - case BO_MulAssign: - case BO_DivAssign: + break; 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. + // For assignment we also allow InitVar = InitVar + N, InitVar = N + + // InitVar, and InitVar = InitVar - N; BUT only if 'N' is integral. + if (!isValidForIncRHSAssign(InitVar, BO->getRHS())) + return DiagIncVar(); break; } IncVar = getDeclFromExpr(BO->getLHS()); @@ -1354,23 +1402,18 @@ bool SemaOpenACC::ForStmtBeginChecker::checkForInc(const Stmt *IncStmt, case OO_MinusMinus: case OO_PlusEqual: case OO_MinusEqual: - case OO_StarEqual: - case OO_SlashEqual: + break; 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. + // For assignment we also allow InitVar = InitVar + N, InitVar = N + + // InitVar, and InitVar = InitVar - N; BUT only if 'N' is integral. + if (!isValidForIncRHSAssign(InitVar, CE->getArg(1))) + return DiagIncVar(); 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. + } else { + return DiagIncVar(); } if (!IncVar) diff --git a/clang/test/SemaOpenACC/loop-construct.cpp b/clang/test/SemaOpenACC/loop-construct.cpp index 7509811635598..0282258023baf 100644 --- a/clang/test/SemaOpenACC/loop-construct.cpp +++ b/clang/test/SemaOpenACC/loop-construct.cpp @@ -30,6 +30,11 @@ struct SomeRAIterator { bool operator!=(SomeRAIterator&); }; +SomeRAIterator &operator+(const SomeRAIterator&, int); +SomeRAIterator &operator+(int,const SomeRAIterator&); +SomeRAIterator &operator-(const SomeRAIterator&, int); +SomeRAIterator &operator-(int,const SomeRAIterator&); + struct HasIteratorCollection { SomeIterator &begin(); SomeIterator &end(); @@ -400,3 +405,37 @@ void inst() { LoopRules<int, int*, float, SomeStruct, SomeIterator, SomeRAIterator>(); } +void allowTrivialAssignStep(int N) { +#pragma acc loop + for(int i = 0; i !=5; i = i + N); +#pragma acc loop + for(int i = 0; i !=5; i = i - N); +#pragma acc loop + for(int i = 0; i !=5; i = N + 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; i = N - 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; i = N + N); + + HasRAIteratorCollection Col; + +#pragma acc loop + for (auto Itr = Col.begin(); Itr != Col.end(); Itr = Itr + N); + +#pragma acc loop + for (auto Itr = Col.begin(); Itr != Col.end(); Itr = Itr - N); + +#pragma acc loop + for (auto Itr = Col.begin(); Itr != Col.end(); Itr = N + Itr); + + // expected-error@+3{{OpenACC 'loop' variable must monotonically increase or decrease}} + // expected-note@+1{{'loop' construct is here}} +#pragma acc loop + for (auto Itr = Col.begin(); Itr != Col.end(); Itr = N - Itr); +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
