Skip to content

Conversation

@Max191
Copy link
Contributor

@Max191 Max191 commented May 20, 2025

When the upper_bound of a gpu dim op (like gpu.block_dim) is the maximum i32 integer value, the op conversion for it causes overflow by adding 1 to convert the bound from closed to open. This fixes the bug by clamping the open bound to the maximum i32 value.

@Max191 Max191 requested a review from krzysz00 May 20, 2025 15:08
@Max191 Max191 requested a review from Mogball as a code owner May 20, 2025 15:08
@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-gpu

Author: None (Max191)

Changes

When the upper_bound of a gpu dim op (like gpu.block_dim) is the maximum i32 integer value, the op conversion for it causes overflow by adding 1 to convert the bound from closed to open. This fixes the bug by clamping the open bound to the MAX_INT value.


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

2 Files Affected:

  • (modified) mlir/lib/Conversion/GPUCommon/IndexIntrinsicsOpLowering.h (+3-1)
  • (modified) mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir (+13)
diff --git a/mlir/lib/Conversion/GPUCommon/IndexIntrinsicsOpLowering.h b/mlir/lib/Conversion/GPUCommon/IndexIntrinsicsOpLowering.h
index 1f158b271e5c6..fa16deae7a93a 100644
--- a/mlir/lib/Conversion/GPUCommon/IndexIntrinsicsOpLowering.h
+++ b/mlir/lib/Conversion/GPUCommon/IndexIntrinsicsOpLowering.h
@@ -116,7 +116,9 @@ struct OpLowering : public ConvertOpToLLVMPattern<Op> {
 
     if (upperBound && intrType != IntrType::None) {
       int32_t min = (intrType == IntrType::Dim ? 1 : 0);
-      int32_t max = *upperBound + (intrType == IntrType::Id ? 0 : 1);
+      int32_t max = *upperBound == INT_MAX
+                        ? *upperBound
+                        : *upperBound + (intrType == IntrType::Id ? 0 : 1);
       newOp->setAttr("range", LLVM::ConstantRangeAttr::get(
                                   rewriter.getContext(), 32, min, max));
     }
diff --git a/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir b/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
index d28aa9e34c22a..e57763c037341 100644
--- a/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
+++ b/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
@@ -763,3 +763,16 @@ gpu.module @test_module {
 gpu.module @test_custom_data_layout attributes {llvm.data_layout = "e"} {
 
 }
+
+// -----
+
+gpu.module @test_module {
+  // CHECK32-LABEL: func @gpu_dim_int_max_upper_bound()
+  func.func @gpu_dim_int_max_upper_bound()
+      -> (index) {
+
+    // CHECK32: rocdl.workgroup.dim.x range <i32, 1, 2147483647> : i32
+    %bDimX = gpu.block_dim x upper_bound 2147483647
+    func.return %bDimX : index
+  }
+}

Signed-off-by: Max Dawkins <[email protected]>
@Max191 Max191 requested a review from adam-smnk May 20, 2025 19:43
@Max191 Max191 merged commit 580f70e into llvm:main May 20, 2025
11 checks passed
@Max191 Max191 deleted the convert-gpu-dims-with-int-limits branch May 20, 2025 23:08
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