Skip to content

Commit

Permalink
[mlir][amdgpu] Remove shared memory optimization pass (#88225)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kuhar committed Apr 11, 2024
1 parent ff74236 commit 4471831
Show file tree
Hide file tree
Showing 20 changed files with 1 addition and 794 deletions.
1 change: 0 additions & 1 deletion mlir/include/mlir/Dialect/AMDGPU/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
add_subdirectory(IR)
add_subdirectory(TransformOps)
add_subdirectory(Transforms)
17 changes: 0 additions & 17 deletions mlir/include/mlir/Dialect/AMDGPU/IR/AMDGPU.td
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,6 @@ def AMDGPU_Dialect : Dialect {
"gpu::GPUDialect"
];
let useDefaultAttributePrinterParser = 1;

let extraClassDeclaration = [{
/// Return true if the given MemRefType has an integer address
/// space that matches the ROCDL shared memory address space or
/// is a gpu::AddressSpaceAttr attribute with value 'workgroup`.
static bool hasSharedMemoryAddressSpace(MemRefType type);

/// Return true if the given Attribute has an integer address
/// space that matches the ROCDL shared memory address space or
/// is a gpu::AddressSpaceAttr attribute with value 'workgroup`.
static bool isSharedMemoryAddressSpace(Attribute type);

/// Defines the MemRef memory space attribute numeric value that indicates
/// a memref is located in shared memory. This should correspond to the
/// value used in ROCDL.
static constexpr unsigned kSharedMemoryAddressSpace = 3;
}];
}

//===----------------------------------------------------------------------===//
Expand Down
48 changes: 0 additions & 48 deletions mlir/include/mlir/Dialect/AMDGPU/TransformOps/AMDGPUTransformOps.h

This file was deleted.

This file was deleted.

4 changes: 0 additions & 4 deletions mlir/include/mlir/Dialect/AMDGPU/TransformOps/CMakeLists.txt

This file was deleted.

2 changes: 1 addition & 1 deletion mlir/include/mlir/Dialect/AMDGPU/Transforms/Passes.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace mlir {
class ConversionTarget;
namespace amdgpu {

#define GEN_PASS_DECL
#define GEN_PASS_DECL_AMDGPUEMULATEATOMICSPASS
#define GEN_PASS_REGISTRATION
#include "mlir/Dialect/AMDGPU/Transforms/Passes.h.inc"

Expand Down
20 changes: 0 additions & 20 deletions mlir/include/mlir/Dialect/AMDGPU/Transforms/Passes.td
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,4 @@ def AmdgpuEmulateAtomicsPass : Pass<"amdgpu-emulate-atomics"> {
"Chipset that these operations will run on">];
}

def OptimizeSharedMemory : Pass<"amdgpu-optimize-shared-memory"> {
let summary = "Optimizes accesses to shared memory memrefs in order to reduce bank conflicts.";
let description = [{
This pass adds a transformation and pass to the AMDGPU dialect that
attempts to optimize reads/writes from a memref representing GPU shared
memory in order to avoid bank conflicts.
}];
let dependentDialects = [
"memref::MemRefDialect", "vector::VectorDialect"
];
let options = [
Option<"sharedMemoryLineSizeBytes", "shared-memory-line-size-bytes", "int64_t",
/*default=*/"128",
"Shared memory line size in bytes">,
Option<"defaultVectorSizeBits", "default-vector-size-bits", "int64_t",
/*default=*/"128",
"Default vector size in bits">,
];
}

#endif // MLIR_DIALECT_AMDGPU_TRANSFORMS_PASSES_TD_
61 changes: 0 additions & 61 deletions mlir/include/mlir/Dialect/AMDGPU/Transforms/Transforms.h

This file was deleted.

24 changes: 0 additions & 24 deletions mlir/include/mlir/Dialect/AMDGPU/Transforms/Utils.h

This file was deleted.

2 changes: 0 additions & 2 deletions mlir/include/mlir/InitAllExtensions.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "mlir/Conversion/MemRefToLLVM/MemRefToLLVM.h"
#include "mlir/Conversion/NVVMToLLVM/NVVMToLLVM.h"
#include "mlir/Conversion/UBToLLVM/UBToLLVM.h"
#include "mlir/Dialect/AMDGPU/TransformOps/AMDGPUTransformOps.h"
#include "mlir/Dialect/Affine/TransformOps/AffineTransformOps.h"
#include "mlir/Dialect/Bufferization/TransformOps/BufferizationTransformOps.h"
#include "mlir/Dialect/Func/Extensions/AllExtensions.h"
Expand Down Expand Up @@ -67,7 +66,6 @@ inline void registerAllExtensions(DialectRegistry &registry) {
ub::registerConvertUBToLLVMInterface(registry);

// Register all transform dialect extensions.
amdgpu::registerTransformDialectExtension(registry);
affine::registerTransformDialectExtension(registry);
bufferization::registerTransformDialectExtension(registry);
func::registerTransformDialectExtension(registry);
Expand Down
1 change: 0 additions & 1 deletion mlir/lib/Dialect/AMDGPU/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
add_subdirectory(IR)
add_subdirectory(TransformOps)
add_subdirectory(Transforms)
add_subdirectory(Utils)
15 changes: 0 additions & 15 deletions mlir/lib/Dialect/AMDGPU/IR/AMDGPUDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,6 @@ void AMDGPUDialect::initialize() {
>();
}

bool amdgpu::AMDGPUDialect::isSharedMemoryAddressSpace(Attribute memorySpace) {
if (!memorySpace)
return false;
if (auto intAttr = llvm::dyn_cast<IntegerAttr>(memorySpace))
return intAttr.getInt() == AMDGPUDialect::kSharedMemoryAddressSpace;
if (auto gpuAttr = llvm::dyn_cast<gpu::AddressSpaceAttr>(memorySpace))
return gpuAttr.getValue() == gpu::AddressSpace::Workgroup;
return false;
}

bool amdgpu::AMDGPUDialect::hasSharedMemoryAddressSpace(MemRefType type) {
Attribute memorySpace = type.getMemorySpace();
return isSharedMemoryAddressSpace(memorySpace);
}

//===----------------------------------------------------------------------===//
// 8-bit float ops
//===----------------------------------------------------------------------===//
Expand Down
67 changes: 0 additions & 67 deletions mlir/lib/Dialect/AMDGPU/TransformOps/AMDGPUTransformOps.cpp

This file was deleted.

25 changes: 0 additions & 25 deletions mlir/lib/Dialect/AMDGPU/TransformOps/CMakeLists.txt

This file was deleted.

Loading

0 comments on commit 4471831

Please sign in to comment.