Skip to content

Conversation

@Hardcode84
Copy link
Contributor

Extracted from #452

This pass expects IR with gpu.launch_func ops and converts it to wave runtime lib calls.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new transformation pass that converts GPU dialect operations (specifically gpu.launch_func) into Wave runtime library calls. The pass expects IR containing gpu.launch_func ops and transforms them into explicit calls to wave_load_kernel and wave_launch_kernel functions.

Key changes:

  • Adds the WaterGPUtoGPURuntime pass that lowers gpu.launch_func operations to runtime calls
  • Registers the ROCDL dialect in water-opt to support #rocdl.target attributes in GPU binary objects
  • Includes comprehensive tests covering basic functionality, multiple kernel launches, and error validation

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
water/lib/Transforms/GPUToGPURuntime.cpp Implements the GPU to runtime pass with kernel loading and launching logic
water/include/water/Transforms/Passes.td Defines the new water-gpu-to-gpu-runtime pass
water/lib/Transforms/CMakeLists.txt Adds build configuration for the new pass and GPU/LLVM dialect dependencies
water/tools/water-opt/water-opt.cpp Registers ROCDLDialect to support rocdl.target attributes used in tests
water/test/Transforms/gpu-to-gpu-runtime.mlir Provides comprehensive test coverage including error cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 193 to 197
std::string strVal = str.str();
strVal.append(std::string_view("\0", 1));
return LLVM::createGlobalString(
loc, builder, getUniqueLLVMGlobalName(mod, symbolTable, varName),
strVal, LLVM::Linkage::Internal);
Copy link
Contributor

@tgymnich tgymnich Nov 26, 2025

Choose a reason for hiding this comment

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

Suggested change
std::string strVal = str.str();
strVal.append(std::string_view("\0", 1));
return LLVM::createGlobalString(
loc, builder, getUniqueLLVMGlobalName(mod, symbolTable, varName),
strVal, LLVM::Linkage::Internal);
Twine strVal = str + "\0"
return LLVM::createGlobalString(
loc, builder, getUniqueLLVMGlobalName(mod, symbolTable, varName),
strVal.str(), LLVM::Linkage::Internal);

use Twine instead of std::string_view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't work this way as Twine interprets "\0" just as empty string, need to wrap into StringRef

Copilot AI review requested due to automatic review settings November 26, 2025 18:01
Copilot finished reviewing on behalf of Hardcode84 November 26, 2025 18:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Copilot AI review requested due to automatic review settings November 26, 2025 21:43
@Hardcode84 Hardcode84 force-pushed the water-gpu-to-gpu-runtime branch from 021e0a2 to 6a25b3c Compare November 26, 2025 21:43
Copilot finished reviewing on behalf of Hardcode84 November 26, 2025 21:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mlir::LLVM::LLVMDialect, mlir::memref::MemRefDialect,
mlir::scf::SCFDialect, mlir::vector::VectorDialect,
wave::WaveDialect>();
mlir::LLVM::LLVMDialect, mlir::ROCDL::ROCDLDialect,
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect you also need to modify cmake to link the dialect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add it, but water-opt is already linking with all dialects via get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)

Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
@Hardcode84 Hardcode84 merged commit 398f6ac into iree-org:main Nov 27, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants