Skip to content

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