Skip to content

[flang][cuda] fix parsing of cuda_kernel #89613

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
Apr 22, 2024

Conversation

ImanHosseini
Copy link
Contributor

@ImanHosseini ImanHosseini commented Apr 22, 2024

Fix parsing of cuda_kernel: it missed a mlir::succeeded check and it was not setting up the types and causing mismatch between values and types of the grid/block (CUFKernelValues). @clementval

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Apr 22, 2024
@ImanHosseini ImanHosseini changed the title fix parsing of cuda_kernel [flang] [parser] fix parsing of cuda_kernel Apr 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 22, 2024

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

Author: Iman Hosseini (ImanHosseini)

Changes

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

2 Files Affected:

  • (modified) flang/lib/Optimizer/Dialect/FIROps.cpp (+9-2)
  • (modified) flang/test/Lower/CUDA/cuda-kernel-loop-directive.cuf (+1)
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index cc08f29a98f022..83ee243d351c02 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -3904,10 +3904,11 @@ mlir::ParseResult parseCUFKernelValues(
     mlir::OpAsmParser &parser,
     llvm::SmallVectorImpl<mlir::OpAsmParser::UnresolvedOperand> &values,
     llvm::SmallVectorImpl<mlir::Type> &types) {
-  if (mlir::succeeded(parser.parseOptionalStar()))
+  if (mlir::succeeded(parser.parseOptionalStar())) {
     return mlir::success();
+  }
 
-  if (parser.parseOptionalLParen()) {
+  if (mlir::succeeded(parser.parseOptionalLParen())) {
     if (mlir::failed(parser.parseCommaSeparatedList(
             mlir::AsmParser::Delimiter::None, [&]() {
               if (parser.parseOperand(values.emplace_back()))
@@ -3915,11 +3916,17 @@ mlir::ParseResult parseCUFKernelValues(
               return mlir::success();
             })))
       return mlir::failure();
+    auto builder = parser.getBuilder();
+    for (size_t i = 0; i < values.size(); i++) {
+      types.emplace_back(builder.getType<mlir::IntegerType>(32));
+    }
     if (parser.parseRParen())
       return mlir::failure();
   } else {
     if (parser.parseOperand(values.emplace_back()))
       return mlir::failure();
+    auto builder = parser.getBuilder();
+    types.emplace_back(builder.getType<mlir::IntegerType>(32));
     return mlir::success();
   }
   return mlir::success();
diff --git a/flang/test/Lower/CUDA/cuda-kernel-loop-directive.cuf b/flang/test/Lower/CUDA/cuda-kernel-loop-directive.cuf
index 6179e609db383c..9b728cd19eb552 100644
--- a/flang/test/Lower/CUDA/cuda-kernel-loop-directive.cuf
+++ b/flang/test/Lower/CUDA/cuda-kernel-loop-directive.cuf
@@ -1,4 +1,5 @@
 ! RUN: bbc -emit-hlfir -fcuda %s -o - | FileCheck %s
+! RUN: bbc -emit-hlfir -fcuda %s -o - | fir-opt | FileCheck %s
 
 ! Test lowering of CUDA kernel loop directive.
 

@ImanHosseini ImanHosseini changed the title [flang] [parser] fix parsing of cuda_kernel [flang] [cuda] fix parsing of cuda_kernel Apr 22, 2024
Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

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

Just couple of nits for the style. Otherwise LGTM.

@clementval clementval changed the title [flang] [cuda] fix parsing of cuda_kernel [flang][cuda] fix parsing of cuda_kernel Apr 22, 2024
ImanHosseini and others added 3 commits April 22, 2024 17:05
Co-authored-by: Valentin Clement (バレンタイン クレメン) <[email protected]>
Co-authored-by: Valentin Clement (バレンタイン クレメン) <[email protected]>
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