Skip to content

Commit 4471831

Browse files
authored
[mlir][amdgpu] Remove shared memory optimization pass (#88225)
This implementation has a number of issues and ultimately does not work on gfx9. * It does not reduce bank conflicts with wide memory accesses. * It does not correctly account for when LDS bank conflicts occur on amdgpu. * The implementation is too fragile to be used on real-world code. For example, the code bails out on any `memref.subview` in the root op, even when the subview is not a user of any of the `memref.alloc` ops. I do not see how these can be easily fixed, therefore I think it's better to delete this code.
1 parent ff74236 commit 4471831

File tree

20 files changed

+1
-794
lines changed

20 files changed

+1
-794
lines changed
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
11
add_subdirectory(IR)
2-
add_subdirectory(TransformOps)
32
add_subdirectory(Transforms)

mlir/include/mlir/Dialect/AMDGPU/IR/AMDGPU.td

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,6 @@ def AMDGPU_Dialect : Dialect {
2929
"gpu::GPUDialect"
3030
];
3131
let useDefaultAttributePrinterParser = 1;
32-
33-
let extraClassDeclaration = [{
34-
/// Return true if the given MemRefType has an integer address
35-
/// space that matches the ROCDL shared memory address space or
36-
/// is a gpu::AddressSpaceAttr attribute with value 'workgroup`.
37-
static bool hasSharedMemoryAddressSpace(MemRefType type);
38-
39-
/// Return true if the given Attribute has an integer address
40-
/// space that matches the ROCDL shared memory address space or
41-
/// is a gpu::AddressSpaceAttr attribute with value 'workgroup`.
42-
static bool isSharedMemoryAddressSpace(Attribute type);
43-
44-
/// Defines the MemRef memory space attribute numeric value that indicates
45-
/// a memref is located in shared memory. This should correspond to the
46-
/// value used in ROCDL.
47-
static constexpr unsigned kSharedMemoryAddressSpace = 3;
48-
}];
4932
}
5033

5134
//===----------------------------------------------------------------------===//

mlir/include/mlir/Dialect/AMDGPU/TransformOps/AMDGPUTransformOps.h

Lines changed: 0 additions & 48 deletions
This file was deleted.

mlir/include/mlir/Dialect/AMDGPU/TransformOps/AMDGPUTransformOps.td

Lines changed: 0 additions & 47 deletions
This file was deleted.

mlir/include/mlir/Dialect/AMDGPU/TransformOps/CMakeLists.txt

Lines changed: 0 additions & 4 deletions
This file was deleted.

mlir/include/mlir/Dialect/AMDGPU/Transforms/Passes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace mlir {
2020
class ConversionTarget;
2121
namespace amdgpu {
2222

23-
#define GEN_PASS_DECL
23+
#define GEN_PASS_DECL_AMDGPUEMULATEATOMICSPASS
2424
#define GEN_PASS_REGISTRATION
2525
#include "mlir/Dialect/AMDGPU/Transforms/Passes.h.inc"
2626

mlir/include/mlir/Dialect/AMDGPU/Transforms/Passes.td

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,4 @@ def AmdgpuEmulateAtomicsPass : Pass<"amdgpu-emulate-atomics"> {
3030
"Chipset that these operations will run on">];
3131
}
3232

33-
def OptimizeSharedMemory : Pass<"amdgpu-optimize-shared-memory"> {
34-
let summary = "Optimizes accesses to shared memory memrefs in order to reduce bank conflicts.";
35-
let description = [{
36-
This pass adds a transformation and pass to the AMDGPU dialect that
37-
attempts to optimize reads/writes from a memref representing GPU shared
38-
memory in order to avoid bank conflicts.
39-
}];
40-
let dependentDialects = [
41-
"memref::MemRefDialect", "vector::VectorDialect"
42-
];
43-
let options = [
44-
Option<"sharedMemoryLineSizeBytes", "shared-memory-line-size-bytes", "int64_t",
45-
/*default=*/"128",
46-
"Shared memory line size in bytes">,
47-
Option<"defaultVectorSizeBits", "default-vector-size-bits", "int64_t",
48-
/*default=*/"128",
49-
"Default vector size in bits">,
50-
];
51-
}
52-
5333
#endif // MLIR_DIALECT_AMDGPU_TRANSFORMS_PASSES_TD_

mlir/include/mlir/Dialect/AMDGPU/Transforms/Transforms.h

Lines changed: 0 additions & 61 deletions
This file was deleted.

mlir/include/mlir/Dialect/AMDGPU/Transforms/Utils.h

Lines changed: 0 additions & 24 deletions
This file was deleted.

mlir/include/mlir/InitAllExtensions.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include "mlir/Conversion/MemRefToLLVM/MemRefToLLVM.h"
2424
#include "mlir/Conversion/NVVMToLLVM/NVVMToLLVM.h"
2525
#include "mlir/Conversion/UBToLLVM/UBToLLVM.h"
26-
#include "mlir/Dialect/AMDGPU/TransformOps/AMDGPUTransformOps.h"
2726
#include "mlir/Dialect/Affine/TransformOps/AffineTransformOps.h"
2827
#include "mlir/Dialect/Bufferization/TransformOps/BufferizationTransformOps.h"
2928
#include "mlir/Dialect/Func/Extensions/AllExtensions.h"
@@ -67,7 +66,6 @@ inline void registerAllExtensions(DialectRegistry &registry) {
6766
ub::registerConvertUBToLLVMInterface(registry);
6867

6968
// Register all transform dialect extensions.
70-
amdgpu::registerTransformDialectExtension(registry);
7169
affine::registerTransformDialectExtension(registry);
7270
bufferization::registerTransformDialectExtension(registry);
7371
func::registerTransformDialectExtension(registry);
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
add_subdirectory(IR)
2-
add_subdirectory(TransformOps)
32
add_subdirectory(Transforms)
43
add_subdirectory(Utils)

mlir/lib/Dialect/AMDGPU/IR/AMDGPUDialect.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,21 +43,6 @@ void AMDGPUDialect::initialize() {
4343
>();
4444
}
4545

46-
bool amdgpu::AMDGPUDialect::isSharedMemoryAddressSpace(Attribute memorySpace) {
47-
if (!memorySpace)
48-
return false;
49-
if (auto intAttr = llvm::dyn_cast<IntegerAttr>(memorySpace))
50-
return intAttr.getInt() == AMDGPUDialect::kSharedMemoryAddressSpace;
51-
if (auto gpuAttr = llvm::dyn_cast<gpu::AddressSpaceAttr>(memorySpace))
52-
return gpuAttr.getValue() == gpu::AddressSpace::Workgroup;
53-
return false;
54-
}
55-
56-
bool amdgpu::AMDGPUDialect::hasSharedMemoryAddressSpace(MemRefType type) {
57-
Attribute memorySpace = type.getMemorySpace();
58-
return isSharedMemoryAddressSpace(memorySpace);
59-
}
60-
6146
//===----------------------------------------------------------------------===//
6247
// 8-bit float ops
6348
//===----------------------------------------------------------------------===//

mlir/lib/Dialect/AMDGPU/TransformOps/AMDGPUTransformOps.cpp

Lines changed: 0 additions & 67 deletions
This file was deleted.

mlir/lib/Dialect/AMDGPU/TransformOps/CMakeLists.txt

Lines changed: 0 additions & 25 deletions
This file was deleted.

0 commit comments

Comments
 (0)