-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flang][cuda] Add restriction on implicit data transfer #87720
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In section 3.4.2, some example of illegal data transfer using expression are given. One of it is when multiple device objects are part of an expression in the rhs. Current implementation allow a single device object in such case.
llvmbot
added
flang
Flang issues not falling into any other category
flang:semantics
labels
Apr 4, 2024
@llvm/pr-subscribers-flang-semantics Author: Valentin Clement (バレンタイン クレメン) (clementval) ChangesIn section 3.4.2, some example of illegal data transfer using expression are given. One of it is when multiple device objects are part of an expression in the rhs. Current implementation allow a single device object in such case. Full diff: https://github.com/llvm/llvm-project/pull/87720.diff 3 Files Affected:
diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index 8c872a0579c8ed..75498f0f685844 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -1226,18 +1226,24 @@ bool CheckForCoindexedObject(parser::ContextualMessages &,
const std::optional<ActualArgument> &, const std::string &procName,
const std::string &argName);
-/// Check if any of the symbols part of the expression has a cuda data
-/// attribute.
-inline bool HasCUDAAttrs(const Expr<SomeType> &expr) {
+// Get the number of symbols with CUDA attribute in the expression.
+template <typename A> inline unsigned GetNbOfCUDASymbols(const A &expr) {
+ unsigned n{0};
for (const Symbol &sym : CollectSymbols(expr)) {
if (const auto *details =
sym.GetUltimate().detailsIf<semantics::ObjectEntityDetails>()) {
if (details->cudaDataAttr()) {
- return true;
+ ++n;
}
}
}
- return false;
+ return n;
+}
+
+// Check if any of the symbols part of the expression has a CUDA data
+// attribute.
+template <typename A> inline bool HasCUDAAttrs(const A &expr) {
+ return GetNbOfCUDASymbols(expr) > 0;
}
} // namespace Fortran::evaluate
diff --git a/flang/lib/Semantics/check-cuda.cpp b/flang/lib/Semantics/check-cuda.cpp
index c0c6ff4c1a2ba3..c0b4e53f30c08c 100644
--- a/flang/lib/Semantics/check-cuda.cpp
+++ b/flang/lib/Semantics/check-cuda.cpp
@@ -9,12 +9,14 @@
#include "check-cuda.h"
#include "flang/Common/template.h"
#include "flang/Evaluate/fold.h"
+#include "flang/Evaluate/tools.h"
#include "flang/Evaluate/traverse.h"
#include "flang/Parser/parse-tree-visitor.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Parser/tools.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/symbol.h"
+#include "flang/Semantics/tools.h"
// Once labeled DO constructs have been canonicalized and their parse subtrees
// transformed into parser::DoConstructs, scan the parser::Blocks of the program
@@ -413,4 +415,18 @@ void CUDAChecker::Enter(const parser::CUFKernelDoConstruct &x) {
}
}
+void CUDAChecker::Enter(const parser::AssignmentStmt &x) {
+ const evaluate::Assignment *assign = semantics::GetAssignment(x);
+ unsigned nbLhs = evaluate::GetNbOfCUDASymbols(assign->lhs);
+ unsigned nbRhs = evaluate::GetNbOfCUDASymbols(assign->rhs);
+ auto lhsLoc{std::get<parser::Variable>(x.t).GetSource()};
+
+ // device to host transfer with more than one device object on the rhs is not
+ // legal.
+ if (nbLhs == 0 && nbRhs > 1) {
+ context_.Say(lhsLoc,
+ "More than one reference to a CUDA object on the right hand side of the assigment"_err_en_US);
+ }
+}
+
} // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/check-cuda.h b/flang/lib/Semantics/check-cuda.h
index d863795f16a7c2..aa0cb46360bef5 100644
--- a/flang/lib/Semantics/check-cuda.h
+++ b/flang/lib/Semantics/check-cuda.h
@@ -17,6 +17,7 @@ struct Program;
class Messages;
struct Name;
class CharBlock;
+struct AssignmentStmt;
struct ExecutionPartConstruct;
struct ExecutableConstruct;
struct ActionStmt;
@@ -38,6 +39,7 @@ class CUDAChecker : public virtual BaseChecker {
void Enter(const parser::FunctionSubprogram &);
void Enter(const parser::SeparateModuleSubprogram &);
void Enter(const parser::CUFKernelDoConstruct &);
+ void Enter(const parser::AssignmentStmt &);
private:
SemanticsContext &context_;
|
klausler
reviewed
Apr 4, 2024
jeanPerier
approved these changes
Apr 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In section 3.4.2, some example of illegal data transfer using expression are given. One of it is when multiple device objects are part of an expression in the rhs. Current implementation allow a single device object in such case. This patch adds a similar restriction.