Skip to content
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] Update memory effect on fir.cuda_allocate op #88930

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

clementval
Copy link
Contributor

Add MemRead effect on the box operand as the descriptor might be read when performing the allocation of the data.

Also update the expected type of the box operand to be a reference. Check in the verifier that this is a reference to a box or class type.

This addresses the comment made post commit on #88586

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Apr 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 16, 2024

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

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

Changes

Add MemRead effect on the box operand as the descriptor might be read when performing the allocation of the data.

Also update the expected type of the box operand to be a reference. Check in the verifier that this is a reference to a box or class type.

This addresses the comment made post commit on #88586


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

3 Files Affected:

  • (modified) flang/include/flang/Optimizer/Dialect/FIROps.td (+1-1)
  • (modified) flang/lib/Optimizer/Dialect/FIROps.cpp (+1-1)
  • (modified) flang/test/Fir/cuf-invalid.fir (+1-1)
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index c181c7ed62dff3..18fdd54f6863ad 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -3200,7 +3200,7 @@ def fir_CUDAAllocateOp : fir_Op<"cuda_allocate", [AttrSizedOperandSegments,
     is initialized before with the standard flang runtime calls.
   }];
 
-  let arguments = (ins Arg<AnyRefOrBoxType, "", [MemWrite]>:$box,
+  let arguments = (ins Arg<fir_ReferenceType, "", [MemRead, MemWrite]>:$box,
                        Arg<Optional<AnyRefOrBoxType>, "", [MemWrite]>:$errmsg,
                        Optional<AnyIntegerType>:$stream,
                        Arg<Optional<AnyRefOrBoxType>, "", [MemWrite]>:$pinned,
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 88710880174d21..df90e2932922cb 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -3998,7 +3998,7 @@ mlir::LogicalResult fir::CUDAAllocateOp::verify() {
     return emitOpError("pinned and stream cannot appears at the same time");
   if (!fir::unwrapRefType(getBox().getType()).isa<fir::BaseBoxType>())
     return emitOpError(
-        "expect box to be a reference to/or a class or box type value");
+        "expect box to be a reference to a class or box type value");
   if (getSource() &&
       !fir::unwrapRefType(getSource().getType()).isa<fir::BaseBoxType>())
     return emitOpError(
diff --git a/flang/test/Fir/cuf-invalid.fir b/flang/test/Fir/cuf-invalid.fir
index 9c5ffe7176a3bd..5bd3c0cd4b4d4c 100644
--- a/flang/test/Fir/cuf-invalid.fir
+++ b/flang/test/Fir/cuf-invalid.fir
@@ -16,7 +16,7 @@ func.func @_QPsub1() {
 
 func.func @_QPsub1() {
   %1 = fir.alloca i32
-  // expected-error@+1{{'fir.cuda_allocate' op expect box to be a reference to/or a class or box type value}}
+  // expected-error@+1{{'fir.cuda_allocate' op expect box to be a reference to a class or box type value}}
   %2 = fir.cuda_allocate %1 : !fir.ref<i32> {cuda_attr = #fir.cuda<device>} -> i32
   return
 }

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

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

Thank you for addressing my comments!

@clementval clementval merged commit a88ea8f into llvm:main Apr 17, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants