Skip to content

[MILR][NVVM] Fix missing semicolon in nvvm.barrier.arrive Op #92769

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 1 commit into from
May 20, 2024

Conversation

schwarzschild-radius
Copy link
Contributor

This commit fixes the missing semicolon in the PTX codegen path where barrier id is provided for the NVVM BarrierArriveOp. Also, updated nvvm-to-llvm.mlir lit test to reflect the same

This commit fixes the missing semicolon in the PTX codegen path where barrier id is
provided for the NVVM BarrierArriveOp. Also, updated nvvm-to-llvm.mlir lit
test to reflect the same
@llvmbot
Copy link
Member

llvmbot commented May 20, 2024

@llvm/pr-subscribers-mlir

Author: Pradeep Kumar (schwarzschild-radius)

Changes

This commit fixes the missing semicolon in the PTX codegen path where barrier id is provided for the NVVM BarrierArriveOp. Also, updated nvvm-to-llvm.mlir lit test to reflect the same


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

2 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td (+1-1)
  • (modified) mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir (+1-1)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td
index 7ffbc2d7922f6..4daeeab093863 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td
@@ -429,7 +429,7 @@ def NVVM_BarrierArriveOp : NVVM_PTXBuilder_Op<"barrier.arrive">
   let extraClassDefinition = [{
     std::string $cppClass::getPtx() {
       std::string ptx = "bar.arrive ";
-      if (getBarrierId()) { ptx += "%0, %1"; } 
+      if (getBarrierId()) { ptx += "%0, %1;"; } 
       else { ptx += "0, %0;"; }
       return ptx;
     }
diff --git a/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir b/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir
index 802760f8c899e..1d56ca97b737e 100644
--- a/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir
+++ b/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir
@@ -688,7 +688,7 @@ func.func @fence_proxy() {
 llvm.func @llvm_nvvm_barrier_arrive(%barID : i32, %numberOfThreads : i32) {
   // CHECK: llvm.inline_asm has_side_effects asm_dialect = att "bar.arrive 0, $0;", "r" %[[numberOfThreads]] : (i32) -> ()
   nvvm.barrier.arrive number_of_threads = %numberOfThreads
-  // CHECK: llvm.inline_asm has_side_effects asm_dialect = att "bar.arrive $0, $1", "r,r" %[[barId]], %[[numberOfThreads]] : (i32, i32) -> ()
+  // CHECK: llvm.inline_asm has_side_effects asm_dialect = att "bar.arrive $0, $1;", "r,r" %[[barId]], %[[numberOfThreads]] : (i32, i32) -> ()
   nvvm.barrier.arrive id = %barID number_of_threads = %numberOfThreads
   llvm.return
 }

@llvmbot
Copy link
Member

llvmbot commented May 20, 2024

@llvm/pr-subscribers-mlir-llvm

Author: Pradeep Kumar (schwarzschild-radius)

Changes

This commit fixes the missing semicolon in the PTX codegen path where barrier id is provided for the NVVM BarrierArriveOp. Also, updated nvvm-to-llvm.mlir lit test to reflect the same


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

2 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td (+1-1)
  • (modified) mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir (+1-1)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td
index 7ffbc2d7922f6..4daeeab093863 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td
@@ -429,7 +429,7 @@ def NVVM_BarrierArriveOp : NVVM_PTXBuilder_Op<"barrier.arrive">
   let extraClassDefinition = [{
     std::string $cppClass::getPtx() {
       std::string ptx = "bar.arrive ";
-      if (getBarrierId()) { ptx += "%0, %1"; } 
+      if (getBarrierId()) { ptx += "%0, %1;"; } 
       else { ptx += "0, %0;"; }
       return ptx;
     }
diff --git a/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir b/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir
index 802760f8c899e..1d56ca97b737e 100644
--- a/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir
+++ b/mlir/test/Conversion/NVVMToLLVM/nvvm-to-llvm.mlir
@@ -688,7 +688,7 @@ func.func @fence_proxy() {
 llvm.func @llvm_nvvm_barrier_arrive(%barID : i32, %numberOfThreads : i32) {
   // CHECK: llvm.inline_asm has_side_effects asm_dialect = att "bar.arrive 0, $0;", "r" %[[numberOfThreads]] : (i32) -> ()
   nvvm.barrier.arrive number_of_threads = %numberOfThreads
-  // CHECK: llvm.inline_asm has_side_effects asm_dialect = att "bar.arrive $0, $1", "r,r" %[[barId]], %[[numberOfThreads]] : (i32, i32) -> ()
+  // CHECK: llvm.inline_asm has_side_effects asm_dialect = att "bar.arrive $0, $1;", "r,r" %[[barId]], %[[numberOfThreads]] : (i32, i32) -> ()
   nvvm.barrier.arrive id = %barID number_of_threads = %numberOfThreads
   llvm.return
 }

@schwarzschild-radius
Copy link
Contributor Author

@joker-eph Thanks for the quick review! :)
Can you please merge the commit? I don't have merge access

@xgupta xgupta merged commit 9964c2c into llvm:main May 20, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants