Skip to content

[mlir][gpu] Refactor GpuOpsToROCDLOps pass interface (NFC) #157402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 10, 2025
Merged

[mlir][gpu] Refactor GpuOpsToROCDLOps pass interface (NFC) #157402

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c03637b
[mlir][gpu] GPUToROCDL: Add C++ argument to populate allowedDialects
pabloantoniom Sep 8, 2025
5572996
[Refactor] Force users to use create* function autogenerated from tab…
pabloantoniom Sep 8, 2025
5dbb951
Cleanup GPUToROCDLPass header
pabloantoniom Sep 9, 2025
a118652
Delete constructors and use the ones from base
pabloantoniom Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions mlir/include/mlir/Conversion/GPUToROCDL/GPUToROCDLPass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@

#include "mlir/Conversion/GPUToROCDL/Runtimes.h"
#include "mlir/Conversion/LLVMCommon/LoweringOptions.h"
#include "mlir/Pass/Pass.h"
#include "llvm/ADT/DenseSet.h"
#include <cstddef>
Copy link
Collaborator

@joker-eph joker-eph Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now you're only removing something from the header, so I'm not sure why you need to add new includes?

Copy link
Contributor Author

@pabloantoniom pabloantoniom Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I forgot to remove those after deleting the constructor in the .td. Only Pass is needed, since now TableGen is creating the create* function which uses mlir::Pass, but I have removed the include and added Pass to the namespace below instead.

#include <memory>

namespace mlir {
Expand Down Expand Up @@ -42,16 +45,6 @@ void populateGpuToROCDLConversionPatterns(const LLVMTypeConverter &converter,
/// Configure target to convert from the GPU dialect to ROCDL.
void configureGpuToROCDLConversionLegality(ConversionTarget &target);

/// Creates a pass that lowers GPU dialect operations to ROCDL counterparts. The
/// index bitwidth used for the lowering of the device side index computations
/// is configurable.
std::unique_ptr<OperationPass<gpu::GPUModuleOp>>
createLowerGpuOpsToROCDLOpsPass(
const std::string &chipset = "gfx900",
unsigned indexBitwidth = kDeriveIndexBitwidthFromDataLayout,
bool useBarePtrCallConv = false,
gpu::amd::Runtime runtime = gpu::amd::Runtime::Unknown);

} // namespace mlir

#endif // MLIR_CONVERSION_GPUTOROCDL_GPUTOROCDLPASS_H_
1 change: 0 additions & 1 deletion mlir/include/mlir/Conversion/Passes.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,6 @@ def ConvertGpuOpsToNVVMOps : Pass<"convert-gpu-to-nvvm", "gpu::GPUModuleOp"> {

def ConvertGpuOpsToROCDLOps : Pass<"convert-gpu-to-rocdl", "gpu::GPUModuleOp"> {
let summary = "Generate ROCDL operations for gpu operations";
let constructor = "mlir::createLowerGpuOpsToROCDLOpsPass()";
let dependentDialects = [
"ROCDL::ROCDLDialect",
"amdgpu::AMDGPUDialect",
Expand Down
25 changes: 12 additions & 13 deletions mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "mlir/Dialect/MemRef/IR/MemRef.h"
#include "mlir/Dialect/Vector/IR/VectorOps.h"
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/Pass/Pass.h"
#include "mlir/Transforms/DialectConversion.h"
#include "mlir/Transforms/GreedyPatternRewriteDriver.h"

Expand Down Expand Up @@ -288,9 +287,12 @@ struct GPUShuffleOpLowering : public ConvertOpToLLVMPattern<gpu::ShuffleOp> {
struct LowerGpuOpsToROCDLOpsPass final
: public impl::ConvertGpuOpsToROCDLOpsBase<LowerGpuOpsToROCDLOpsPass> {
LowerGpuOpsToROCDLOpsPass() = default;
LowerGpuOpsToROCDLOpsPass(const std::string &chipset, unsigned indexBitwidth,
bool useBarePtrCallConv,
gpu::amd::Runtime runtime) {
LowerGpuOpsToROCDLOpsPass(ConvertGpuOpsToROCDLOpsOptions options)
: ConvertGpuOpsToROCDLOpsBase(options) {}
LowerGpuOpsToROCDLOpsPass(
const std::string &chipset, unsigned indexBitwidth,
bool useBarePtrCallConv, gpu::amd::Runtime runtime,
std::optional<llvm::SmallDenseSet<StringRef>> allowedDialects) {
if (this->chipset.getNumOccurrences() == 0)
Copy link
Contributor

@fabianmcg fabianmcg Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need these? Can't we remove?

Copy link
Contributor Author

@pabloantoniom pabloantoniom Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, thanks for the suggestion. By removing this we also match what LowerGpuOpsToNVVMOpsPass does so it should be easier for you if you want to do some cleanup later.

this->chipset = chipset;
if (this->indexBitwidth.getNumOccurrences() == 0)
Expand All @@ -299,6 +301,12 @@ struct LowerGpuOpsToROCDLOpsPass final
this->useBarePtrCallConv = useBarePtrCallConv;
if (this->runtime.getNumOccurrences() == 0)
this->runtime = runtime;
if (this->allowedDialects.getNumOccurrences() == 0 &&
allowedDialects.has_value()) {
for (auto &str : allowedDialects.value()) {
this->allowedDialects.push_back(str.str());
}
}
}

void getDependentDialects(DialectRegistry &registry) const override {
Expand Down Expand Up @@ -499,12 +507,3 @@ void mlir::populateGpuToROCDLConversionPatterns(

populateMathToROCDLConversionPatterns(converter, patterns);
}

std::unique_ptr<OperationPass<gpu::GPUModuleOp>>
mlir::createLowerGpuOpsToROCDLOpsPass(const std::string &chipset,
unsigned indexBitwidth,
bool useBarePtrCallConv,
gpu::amd::Runtime runtime) {
return std::make_unique<LowerGpuOpsToROCDLOpsPass>(
chipset, indexBitwidth, useBarePtrCallConv, runtime);
}