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] Relax the verifier for cuf.register_kernel op #112585

Merged
merged 3 commits into from
Oct 17, 2024

Conversation

clementval
Copy link
Contributor

Relax the verifier since the gpu.func might be converted to llvm.func before cuf.register_kernel is converted.

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

llvmbot commented Oct 16, 2024

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

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

Changes

Relax the verifier since the gpu.func might be converted to llvm.func before cuf.register_kernel is converted.


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

2 Files Affected:

  • (modified) flang/lib/Optimizer/Dialect/CUF/CUFOps.cpp (+6-6)
  • (modified) flang/test/Fir/cuf-invalid.fir (-15)
diff --git a/flang/lib/Optimizer/Dialect/CUF/CUFOps.cpp b/flang/lib/Optimizer/Dialect/CUF/CUFOps.cpp
index 9e3bbd1f9cbee9..5f7232e3dd4b09 100644
--- a/flang/lib/Optimizer/Dialect/CUF/CUFOps.cpp
+++ b/flang/lib/Optimizer/Dialect/CUF/CUFOps.cpp
@@ -281,12 +281,12 @@ mlir::LogicalResult cuf::RegisterKernelOp::verify() {
 
   mlir::SymbolTable gpuSymTab(gpuMod);
   auto func = gpuSymTab.lookup<mlir::gpu::GPUFuncOp>(getKernelName());
-  if (!func)
-    return emitOpError("device function not found");
-
-  if (!func.isKernel())
-    return emitOpError("only kernel gpu.func can be registered");
-
+  if (func) {
+    // Only check if the gpu.func is found. It might be converted to LLVMFuncOp
+    // already.
+    if (!func.isKernel())
+      return emitOpError("only kernel gpu.func can be registered");
+  }
   return mlir::success();
 }
 
diff --git a/flang/test/Fir/cuf-invalid.fir b/flang/test/Fir/cuf-invalid.fir
index a5747b8ee4a3b3..ac488ab3e5abf5 100644
--- a/flang/test/Fir/cuf-invalid.fir
+++ b/flang/test/Fir/cuf-invalid.fir
@@ -143,21 +143,6 @@ module attributes {gpu.container_module} {
 
 // -----
 
-module attributes {gpu.container_module} {
-  gpu.module @cuda_device_mod {
-    gpu.func @_QPsub_device1() {
-      gpu.return
-    }
-  }
-  llvm.func internal @__cudaFortranConstructor() {
-    // expected-error@+1{{'cuf.register_kernel' op device function not found}}
-    cuf.register_kernel @cuda_device_mod::@_QPsub_device2
-    llvm.return
-  }
-}
-
-// -----
-
 module attributes {gpu.container_module} {
   llvm.func internal @__cudaFortranConstructor() {
     // expected-error@+1{{'cuf.register_kernel' op gpu module not found}}

Copy link
Contributor

@Renaud-K Renaud-K 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.

@clementval clementval merged commit 834d001 into llvm:main Oct 17, 2024
8 checks passed
@clementval clementval deleted the cuf_register_verifier_relax branch October 17, 2024 15:30
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.

3 participants