Skip to content

[flang][cuda] Do not lower device variables in main program as globals #102512

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

Merged
merged 4 commits into from
Aug 8, 2024

Conversation

clementval
Copy link
Contributor

Flang considers arrays in main program larger than 32 bytes having the SAVE attribute and lowers them as globals. In CUDA Fortran, device variables are not allowed to have the SAVE attribute and should be allocated dynamically in the main program scope.
This patch updates lowering so CUDA Fortran device variables are not considered with the SAVE attribute.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Aug 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2024

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-fir-hlfir

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

Flang considers arrays in main program larger than 32 bytes having the SAVE attribute and lowers them as globals. In CUDA Fortran, device variables are not allowed to have the SAVE attribute and should be allocated dynamically in the main program scope.
This patch updates lowering so CUDA Fortran device variables are not considered with the SAVE attribute.


Full diff: https://github.com/llvm/llvm-project/pull/102512.diff

2 Files Affected:

  • (modified) flang/lib/Evaluate/tools.cpp (+2-1)
  • (modified) flang/test/Lower/CUDA/cuda-program-global.cuf (+4-3)
diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index 34faba39ffd46f..e042d4d5a09d5d 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1696,7 +1696,8 @@ bool IsSaved(const Symbol &original) {
       (features.IsEnabled(common::LanguageFeature::SaveMainProgram) ||
           (features.IsEnabled(
                common::LanguageFeature::SaveBigMainProgramVariables) &&
-              symbol.size() > 32))) {
+              symbol.size() > 32)) &&
+      !Fortran::evaluate::IsCUDADeviceSymbol(symbol)) {
     // With SaveBigMainProgramVariables, keeping all unsaved main program
     // variables of 32 bytes or less on the stack allows keeping numerical and
     // logical scalars, small scalar characters or derived, small arrays, and
diff --git a/flang/test/Lower/CUDA/cuda-program-global.cuf b/flang/test/Lower/CUDA/cuda-program-global.cuf
index 97b9927b3082fd..498725cd44905f 100644
--- a/flang/test/Lower/CUDA/cuda-program-global.cuf
+++ b/flang/test/Lower/CUDA/cuda-program-global.cuf
@@ -1,6 +1,7 @@
 ! RUN: bbc -emit-hlfir -fcuda %s -o - | FileCheck %s
 
-! Test lowering of program local variable that are global
+! Test lowering of program local variables. Make sure CUDA device variables are
+! not lowered as global.
 
 program test
   integer, device :: a(10)
@@ -10,10 +11,10 @@ program test
 end
 
 ! CHECK-LABEL: func.func @_QQmain()
-! CHECK: fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xi32>>
+! CHECK: cuf.alloc !fir.array<10xi32> {bindc_name = "a", data_attr = #cuf.cuda<device>, uniq_name = "_QFEa"} -> !fir.ref<!fir.array<10xi32>>
 ! CHECK: fir.address_of(@_QFEb) : !fir.ref<!fir.array<10xi32>>
 ! CHECK: %[[ALLOCA:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFEi"}
 ! CHECK: hlfir.declare %[[ALLOCA]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 
-! CHECK: fir.global internal @_QFEa {data_attr = #cuf.cuda<device>} : !fir.array<10xi32> {{{$}}
+! CHECK-NOT: fir.global internal @_QFEa {data_attr = #cuf.cuda<device>} : !fir.array<10xi32> {{{$}}
 ! CHECK: fir.global internal @_QFEb : !fir.array<10xi32> {{{$}}

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link

github-actions bot commented Aug 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link

github-actions bot commented Aug 8, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 6b78e94110dda175b248b1c361a098236522e1e2 54e62780152724e5963fb011fd985d6b58e97200 --extensions cpp,h -- flang/include/flang/Evaluate/tools.h flang/lib/Evaluate/tools.cpp
View the diff from clang-format here.
diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index 1bfd253acc..4d78f814f8 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1715,7 +1715,7 @@ bool IsSaved(const Symbol &original) {
   } else if (symbol.test(Symbol::Flag::InDataStmt)) {
     return true;
   } else if (const auto *object{symbol.detailsIf<ObjectEntityDetails>()};
-      object && object->init()) {
+             object && object->init()) {
     return true;
   } else if (IsProcedurePointer(symbol) && symbol.has<ProcEntityDetails>() &&
       symbol.get<ProcEntityDetails>().init()) {
@@ -1723,7 +1723,7 @@ bool IsSaved(const Symbol &original) {
   } else if (scope.hasSAVE()) {
     return true; // bare SAVE statement
   } else if (const Symbol * block{FindCommonBlockContaining(symbol)};
-      block && block->attrs().test(Attr::SAVE)) {
+             block && block->attrs().test(Attr::SAVE)) {
     return true; // in COMMON with SAVE
   } else {
     return false;

@clementval clementval merged commit 8c3b6bd into llvm:main Aug 8, 2024
6 of 7 checks passed
@clementval clementval deleted the cuf_program_arrays branch August 8, 2024 20:13
clementval added a commit that referenced this pull request Aug 13, 2024
…102996)

Similar to #102512. Do not lower PINNED variable descriptors in program
unit as globals but keep them local.
clementval added a commit that referenced this pull request Dec 16, 2024
As it was done in #102512, do not create global for arrays declared in
program unit with cuda data attribute.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants