-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[flang] [cuda] Fix CUDA implicit data transfer entity creation #139414
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
Conversation
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Zhen Wang (wangzpgi) ChangesFixed an issue in Full diff: https://github.com/llvm/llvm-project/pull/139414.diff 1 Files Affected:
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 43375e84f21fa..bfe8898ebff3d 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -4778,7 +4778,13 @@ class FirConverter : public Fortran::lower::AbstractConverter {
nbDeviceResidentObject <= 1 &&
"Only one reference to the device resident object is supported");
auto addr = getSymbolAddress(sym);
- hlfir::Entity entity{addr};
+ mlir::Value baseValue;
+ if (auto declareOp = llvm::dyn_cast<hlfir::DeclareOp>(addr.getDefiningOp()))
+ baseValue = declareOp.getBase();
+ else
+ baseValue = addr;
+
+ hlfir::Entity entity{baseValue};
auto [temp, cleanup] =
hlfir::createTempFromMold(loc, builder, entity);
auto needCleanup = fir::getIntIfConstant(cleanup);
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
clementval
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make sure your new test check the actual change?
| end subroutine | ||
|
|
||
| !CHECK-LABEL: func.func @_QPtestr2 | ||
| !CHECK: %{{.*}} = cuf.alloc !fir.array<?x?xf32>, %{{.*}}, %{{.*}} : index, index {bindc_name = "ai4", data_attr = #cuf.cuda<managed>, uniq_name = "_QFtestr2Eai4"} -> !fir.ref<!fir.array<?x?xf32>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check the actual change?
clementval
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixed an issue in
genCUDAImplicitDataTransferwhere creating anhlfir::Entityfrom a symbol address could fail when the address comes from ahlfir.declareoperation. Fix is to check if the address comes from ahlfir.declareoperation. If so, use the base value from the declare op when available. Falling back to the original address otherwise.