[mlir][ROCDL] Swap range metadata to range attribute

[andjo403]

Swap out range metadata to range attribute for calls to be able to deprecate range metadata on calls in the future.

[llvmbot]

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Andreas Jonson (andjo403)

Changes

Swap out range metadata to range attribute for calls to be able to deprecate range metadata on calls in the future.

---

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

2 Files Affected:

• (modified) mlir/lib/Target/LLVMIR/Dialect/ROCDL/ROCDLToLLVMIRTranslation.cpp (+4-7)
• (modified) mlir/test/Target/LLVMIR/rocdl.mlir (+1-2)

diff --git a/mlir/lib/Target/LLVMIR/Dialect/ROCDL/ROCDLToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/ROCDL/ROCDLToLLVMIRTranslation.cpp
index 94423b35d1ff3..2a146f5efed30 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/ROCDL/ROCDLToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/ROCDL/ROCDLToLLVMIRTranslation.cpp
@@ -17,9 +17,9 @@
 #include "mlir/IR/Operation.h"
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 
+#include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/IntrinsicsAMDGPU.h"
-#include "llvm/IR/MDBuilder.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace mlir;
@@ -32,12 +32,9 @@ static llvm::Value *createIntrinsicCallWithRange(llvm::IRBuilderBase &builder,
   auto *inst = llvm::cast<llvm::CallInst>(
       createIntrinsicCall(builder, intrinsic, {}, {}));
   if (maybeRange) {
-    SmallVector<llvm::APInt, 2> apInts;
-    for (int32_t i : maybeRange.asArrayRef())
-      apInts.push_back(llvm::APInt(32, i));
-    llvm::MDBuilder mdBuilder(builder.getContext());
-    llvm::MDNode *range = mdBuilder.createRange(apInts[0], apInts[1]);
-    inst->setMetadata(llvm::LLVMContext::MD_range, range);
+    llvm::ConstantRange Range(APInt(32, maybeRange[0]),
+                              APInt(32, maybeRange[1]));
+    inst->addRangeRetAttr(Range);
   }
   return inst;
 }
diff --git a/mlir/test/Target/LLVMIR/rocdl.mlir b/mlir/test/Target/LLVMIR/rocdl.mlir
index 3fd1a5bdbe85e..78c3987fab648 100644
--- a/mlir/test/Target/LLVMIR/rocdl.mlir
+++ b/mlir/test/Target/LLVMIR/rocdl.mlir
@@ -27,7 +27,7 @@ llvm.func @rocdl_special_regs() -> i32 {
   // CHECK: call i64 @__ockl_get_num_groups(i32 2)
   %12 = rocdl.grid.dim.z : i64
 
-  // CHECK: call i32 @llvm.amdgcn.workitem.id.x(),{{.*}} !range ![[$RANGE:[0-9]+]]
+  // CHECK: call range(i32 0, 64) i32 @llvm.amdgcn.workitem.id.x()
   %13 = rocdl.workitem.id.x {range = array<i32: 0, 64>} : i32
 
   llvm.return %1 : i32
@@ -520,5 +520,4 @@ llvm.func @rocdl_8bit_floats(%source: i32, %stoch: i32) -> i32 {
 // CHECK-DAG: attributes #[[$KERNEL_WORKGROUP_ATTRS]] = { "amdgpu-flat-work-group-size"="1,1024"
 // CHECK-DAG: attributes #[[$KNOWN_BLOCK_SIZE_ATTRS]] = { "amdgpu-flat-work-group-size"="128,128"
 // CHECK-DAG: attributes #[[$KERNEL_NO_UNIFORM_WORK_GROUPS_ATTRS]] = { "amdgpu-flat-work-group-size"="1,256" "uniform-work-group-size"="false" }
-// CHECK-DAG: ![[$RANGE]] = !{i32 0, i32 64}
 // CHECK-DAG: ![[$REQD_WORK_GROUP_SIZE]] = !{i32 16, i32 4, i32 2}

[andjo403]

maybe @krzysz00 or @antiagainst that was involved in adding this with the metadata in https://reviews.llvm.org/D139866 want to review this

[krzysz00]

As a matter of not impeding progress on a deprecation, approved.

However, I'd like to ask a refactoring favor while you're here (since I ... just started writing a branch to do this). If it's not too much trouble, could you go ahead and

• Add a RangeOpInterface by analogy to AliasAnalysisOpInterface or FastmathFlagsInterface?
• Make the range translation code part of IntrOpBase, similarly (bonus points for bidirectional)
• Go mark rocdl.workitem_id and its fellows (all the special registers, really) with being operations that can have ranges on them
  • And get the nvvm dialect ones too

[krzysz00]

(I'm also approving because, last I checked, all the places that look for !range in the LLVM frontend , like ValueTracking, know to look for the attribute as well)

[andjo403]

Thanks for the review, sorry to say but I think that the refactoring is more then I can handle right now.
I also do not have commit access so need help with merging.

[krzysz00]

I'll land this US tomorrow morning, then?

[andjo403]

thank you for merging