[clang] ea0e6e3 - [OpenACC] Improve 'loop' content restrictions
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 30 12:02:56 PDT 2025
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 at +3{{OpenACC 'loop' variable must monotonically increase or decrease}}
+ // expected-note at +1{{'loop' construct is here}}
+#pragma acc loop
+ for(int i = 0; i !=5; i = N - i);
+ // expected-error at +3{{OpenACC 'loop' variable must monotonically increase or decrease}}
+ // expected-note at +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 at +3{{OpenACC 'loop' variable must monotonically increase or decrease}}
+ // expected-note at +1{{'loop' construct is here}}
+#pragma acc loop
+ for (auto Itr = Col.begin(); Itr != Col.end(); Itr = N - Itr);
+}
+
More information about the cfe-commits
mailing list