-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[mlir][vector] fix: unroll vector.from_elements in gpu pipelines #154774
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.
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
[mlir][vector] fix: unroll vector.from_elements in gpu pipelines #154774
Conversation
|
@llvm/pr-subscribers-mlir Author: Yang Bai (yangtetris) ChangesProblemPR #142944 introduced a new canonicalization pattern which caused failures in the following GPU-related integration tests:
The issue occurs because the new canonicalization pattern can generate multi-dimensional FixThis PR adds
Full diff: https://github.com/llvm/llvm-project/pull/154774.diff 2 Files Affected:
diff --git a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
index 3cfbd898e49e2..e516118f75207 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
@@ -532,6 +532,9 @@ void GpuToLLVMConversionPass::runOnOperation() {
// Vector transfer ops with rank > 1 should be lowered with VectorToSCF.
vector::populateVectorTransferLoweringPatterns(patterns,
/*maxTransferRank=*/1);
+ // Transform N-D vector.from_elements to 1-D vector.from_elements before
+ // conversion.
+ vector::populateVectorFromElementsLoweringPatterns(patterns);
if (failed(applyPatternsGreedily(getOperation(), std::move(patterns))))
return signalPassFailure();
}
diff --git a/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp b/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
index 317bfc2970cf5..50ac1c60184eb 100644
--- a/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
+++ b/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
@@ -27,6 +27,7 @@
#include "mlir/Dialect/Math/IR/Math.h"
#include "mlir/Dialect/MemRef/IR/MemRef.h"
#include "mlir/Dialect/NVGPU/IR/NVGPUDialect.h"
+#include "mlir/Dialect/Vector/Transforms/LoweringPatterns.h"
#include "mlir/Transforms/DialectConversion.h"
#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
@@ -369,6 +370,9 @@ struct LowerGpuOpsToNVVMOpsPass final
{
RewritePatternSet patterns(m.getContext());
populateGpuRewritePatterns(patterns);
+ // Transform N-D vector.from_elements to 1-D vector.from_elements before
+ // conversion.
+ vector::populateVectorFromElementsLoweringPatterns(patterns);
if (failed(applyPatternsGreedily(m, std::move(patterns))))
return signalPassFailure();
}
|
|
@llvm/pr-subscribers-mlir-gpu Author: Yang Bai (yangtetris) ChangesProblemPR #142944 introduced a new canonicalization pattern which caused failures in the following GPU-related integration tests:
The issue occurs because the new canonicalization pattern can generate multi-dimensional FixThis PR adds
Full diff: https://github.com/llvm/llvm-project/pull/154774.diff 2 Files Affected:
diff --git a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
index 3cfbd898e49e2..e516118f75207 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
@@ -532,6 +532,9 @@ void GpuToLLVMConversionPass::runOnOperation() {
// Vector transfer ops with rank > 1 should be lowered with VectorToSCF.
vector::populateVectorTransferLoweringPatterns(patterns,
/*maxTransferRank=*/1);
+ // Transform N-D vector.from_elements to 1-D vector.from_elements before
+ // conversion.
+ vector::populateVectorFromElementsLoweringPatterns(patterns);
if (failed(applyPatternsGreedily(getOperation(), std::move(patterns))))
return signalPassFailure();
}
diff --git a/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp b/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
index 317bfc2970cf5..50ac1c60184eb 100644
--- a/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
+++ b/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
@@ -27,6 +27,7 @@
#include "mlir/Dialect/Math/IR/Math.h"
#include "mlir/Dialect/MemRef/IR/MemRef.h"
#include "mlir/Dialect/NVGPU/IR/NVGPUDialect.h"
+#include "mlir/Dialect/Vector/Transforms/LoweringPatterns.h"
#include "mlir/Transforms/DialectConversion.h"
#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
@@ -369,6 +370,9 @@ struct LowerGpuOpsToNVVMOpsPass final
{
RewritePatternSet patterns(m.getContext());
populateGpuRewritePatterns(patterns);
+ // Transform N-D vector.from_elements to 1-D vector.from_elements before
+ // conversion.
+ vector::populateVectorFromElementsLoweringPatterns(patterns);
if (failed(applyPatternsGreedily(m, std::move(patterns))))
return signalPassFailure();
}
|
|
Thanks for fixing this. I guess we don't test these failures on pre-merge? |
|
Please help merge this PR at your convenience. Thanks! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/17925 Here is the relevant piece of the build log for the reference |
Problem
PR #142944 introduced a new canonicalization pattern which caused failures in the following GPU-related integration tests:
The issue occurs because the new canonicalization pattern can generate multi-dimensional
vector.from_elementsoperations (rank > 1), but the GPU lowering pipelines were not equipped to handle these during the conversion to LLVM.Fix
This PR adds
vector::populateVectorFromElementsLoweringPatternsto the GPU lowering passes that are integrated ingpu-lower-to-nvvm-pipeline:GpuToLLVMConversionPass: the general GPU-to-LLVM conversion pass.LowerGpuOpsToNVVMOpsPass: the NVVM-specific lowering pass.