Partially revert "[mlir][NVVM] Add constant memory space identifier"

[dklimkin]

The second part of the change introduced circular dependency between LLVMDialect and BasicPtxBuilderInterface.

[llvmbot]

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Danial Klimkin (dklimkin)

Changes

The second part of the change introduced circular dependency between LLVMDialect and BasicPtxBuilderInterface.

---

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

1 Files Affected:

• (modified) mlir/lib/Dialect/LLVMIR/IR/BasicPtxBuilderInterface.cpp (+3-2)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/BasicPtxBuilderInterface.cpp b/mlir/lib/Dialect/LLVMIR/IR/BasicPtxBuilderInterface.cpp
index d181700f757a5b..b109f00c3da13a 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/BasicPtxBuilderInterface.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/BasicPtxBuilderInterface.cpp
@@ -12,7 +12,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/LLVMIR/BasicPtxBuilderInterface.h"
-#include "mlir/Dialect/LLVMIR/NVVMDialect.h"
 
 #define DEBUG_TYPE "ptx-builder"
 #define DBGS() (llvm::dbgs() << "[" DEBUG_TYPE "]: ")
@@ -27,6 +26,8 @@
 using namespace mlir;
 using namespace NVVM;
 
+static constexpr int64_t kSharedMemorySpace = 3;
+
 static char getRegisterType(Type type) {
   if (type.isInteger(1))
     return 'b';
@@ -42,7 +43,7 @@ static char getRegisterType(Type type) {
     return 'd';
   if (auto ptr = dyn_cast<LLVM::LLVMPointerType>(type)) {
     // Shared address spaces is addressed with 32-bit pointers.
-    if (ptr.getAddressSpace() == NVVMMemorySpace::kSharedMemorySpace) {
+    if (ptr.getAddressSpace() == kSharedMemorySpace) {
       return 'r';
     }
     return 'l';

[dklimkin]

https://buildkite.com/llvm-project/upstream-bazel/builds/112567#019257b3-a062-4ea5-a6d5-e41469ca5055

    @@llvm-project//mlir:BasicPtxBuilderInterface (ccbefc8b2e2900f51345b35211e15a5555c9071718feb222721d0da47fc335fd)
.-> @@llvm-project//mlir:BasicPtxBuilderInterface (d338a3ecd0023e40be049a609bc260c8de25852681e63cddc3bab9e8ceae2ce1)
|   @@llvm-project//mlir:NVVMDialect (d338a3ecd0023e40be049a609bc260c8de25852681e63cddc3bab9e8ceae2ce1)
`-- @@llvm-project//mlir:BasicPtxBuilderInterface (d338a3ecd0023e40be049a609bc260c8de25852681e63cddc3bab9e8ceae2ce1)

[bjacob]

@dklimkin , I didn't see this and fixed it in a different way by fusing these two Bazel targets in #111172. Hopefully, this unblocks re-landing the commit that was reverted here.

[bjacob]

FYI @matthias-springer

[joker-eph]

@dklimkin this kind of Bazel issues are most of the time false dependencies, to be fixed in Bazel instead of changing the code.

Thanks @bjacob for the follow up!

[dklimkin]

@joker-eph well, I did try removing dep to see if it's real, the same way @bjacob did. Generally, the fly-by change was not needed for "Add constant memory space identifier".

Thanks for addressing this, Benoit.

[joker-eph]

@dklimkin I don't understand what you mean? What didn't work in " I did try removing dep to see if it's real, the same way @bjacob did" that led to changing the code here?