[Lldb-commits] [llvm] [lld] [lldb] [compiler-rt] [mlir] [clang] [flang] [libcxx] [mlir][async] Avoid crash when not using `func.func` (PR #72801)
https://github.com/rikhuijzer updated
https://github.com/llvm/llvm-project/pull/72801
>From 8abbf36f741c8363155e0f3cbf2450ff7f1f0801 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Sun, 19 Nov 2023 18:31:38 +0100
Subject: [PATCH 1/3] [mlir][async] Avoid crash when not using `func.func`
---
.../Async/Transforms/AsyncParallelFor.cpp | 4
.../Async/async-parallel-for-compute-fn.mlir | 19 +++
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 2 ++
3 files changed, 25 insertions(+)
diff --git a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
index 12a28c2e23b221a..639bc7f9ec7f112 100644
--- a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
+++ b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
@@ -102,6 +102,10 @@ struct AsyncParallelForPass
: public impl::AsyncParallelForBase {
AsyncParallelForPass() = default;
+ void getDependentDialects(DialectRegistry ®istry) const override {
+registry.insert();
+ }
+
AsyncParallelForPass(bool asyncDispatch, int32_t numWorkerThreads,
int32_t minTaskSize) {
this->asyncDispatch = asyncDispatch;
diff --git a/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
b/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
index 2115b1881fa6d66..fa3b53dd839c6c6 100644
--- a/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
+++ b/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
@@ -69,6 +69,25 @@ func.func @sink_constant_step(%arg0: memref, %lb:
index, %ub: index) {
// -
+// Smoke test that parallel for doesn't crash when func dialect is not loaded.
+
+// CHECK-LABEL: llvm.func @without_func_dialect()
+llvm.func @without_func_dialect() {
+ %cst = arith.constant 0.0 : f32
+
+ %c0 = arith.constant 0 : index
+ %c22 = arith.constant 22 : index
+ %c1 = arith.constant 1 : index
+ %54 = memref.alloc() : memref<22xf32>
+ %alloc_4 = memref.alloc() : memref<22xf32>
+ scf.parallel (%arg0) = (%c0) to (%c22) step (%c1) {
+memref.store %cst, %alloc_4[%arg0] : memref<22xf32>
+ }
+ llvm.return
+}
+
+// -
+
// Check that for statically known inner loop bound block size is aligned and
// inner loop uses statically known loop trip counts.
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 842964b853d084d..963c52fd4191657 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1143,6 +1143,8 @@ void OpEmitter::genAttrNameGetters() {
const char *const getAttrName = R"(
assert(index < {0} && "invalid attribute index");
assert(name.getStringRef() == getOperationName() && "invalid operation
name");
+ assert(!name.getAttributeNames().empty() && "empty attribute names. Is a new
"
+ "op created without having initialized its dialect?");
return name.getAttributeNames()[index];
)";
method->body() << formatv(getAttrName, attributes.size());
>From eb09cc895d7d1c08f745df22345cd0fae5432c7a Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Mon, 20 Nov 2023 19:23:49 +0100
Subject: [PATCH 2/3] Declare dependentDialects in `Passes.td`
---
mlir/include/mlir/Dialect/Async/Passes.td | 1 +
mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp | 4
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/mlir/include/mlir/Dialect/Async/Passes.td
b/mlir/include/mlir/Dialect/Async/Passes.td
index c7ee4ba39aecdf0..f0ef83ca3fd4f1a 100644
--- a/mlir/include/mlir/Dialect/Async/Passes.td
+++ b/mlir/include/mlir/Dialect/Async/Passes.td
@@ -36,6 +36,7 @@ def AsyncParallelFor : Pass<"async-parallel-for", "ModuleOp">
{
let dependentDialects = [
"arith::ArithDialect",
"async::AsyncDialect",
+"func::FuncDialect",
"scf::SCFDialect"
];
}
diff --git a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
index 639bc7f9ec7f112..12a28c2e23b221a 100644
--- a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
+++ b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
@@ -102,10 +102,6 @@ struct AsyncParallelForPass
: public impl::AsyncParallelForBase {
AsyncParallelForPass() = default;
- void getDependentDialects(DialectRegistry ®istry) const override {
-registry.insert();
- }
-
AsyncParallelForPass(bool asyncDispatch, int32_t numWorkerThreads,
int32_t minTaskSize) {
this->asyncDispatch = asyncDispatch;
>From 77ba982eba8f7511543e9e06864a15c839feece8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Mon, 20 Nov 2023 21:19:37 +0100
Subject: [PATCH 3/3] Update assertion
---
mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir | 2 +-
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp| 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/mlir/test/Dialect/Async/async-parallel-for-comp
[Lldb-commits] [llvm] [lld] [lldb] [compiler-rt] [mlir] [clang] [flang] [libcxx] [mlir][async] Avoid crash when not using `func.func` (PR #72801)
@@ -1143,6 +1143,8 @@ void OpEmitter::genAttrNameGetters() {
const char *const getAttrName = R"(
assert(index < {0} && "invalid attribute index");
assert(name.getStringRef() == getOperationName() && "invalid operation
name");
+ assert(!name.getAttributeNames().empty() && "empty attribute names. Is a new
"
rikhuijzer wrote:
Good tip. Thanks! I've implemented it and verified that it is triggered when
reverting the fix from this PR (`func::FuncDialect` in `dependentDialects`).
https://github.com/llvm/llvm-project/pull/72801
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [clang-tools-extra] [openmp] [libc] [compiler-rt] [clang] [lld] [flang] [libcxx] [mlir] [lldb] [mlir][async] Avoid crash when not using `func.func` (PR #72801)
https://github.com/rikhuijzer updated
https://github.com/llvm/llvm-project/pull/72801
>From 8abbf36f741c8363155e0f3cbf2450ff7f1f0801 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Sun, 19 Nov 2023 18:31:38 +0100
Subject: [PATCH 1/3] [mlir][async] Avoid crash when not using `func.func`
---
.../Async/Transforms/AsyncParallelFor.cpp | 4
.../Async/async-parallel-for-compute-fn.mlir | 19 +++
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 2 ++
3 files changed, 25 insertions(+)
diff --git a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
index 12a28c2e23b221a..639bc7f9ec7f112 100644
--- a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
+++ b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
@@ -102,6 +102,10 @@ struct AsyncParallelForPass
: public impl::AsyncParallelForBase {
AsyncParallelForPass() = default;
+ void getDependentDialects(DialectRegistry ®istry) const override {
+registry.insert();
+ }
+
AsyncParallelForPass(bool asyncDispatch, int32_t numWorkerThreads,
int32_t minTaskSize) {
this->asyncDispatch = asyncDispatch;
diff --git a/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
b/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
index 2115b1881fa6d66..fa3b53dd839c6c6 100644
--- a/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
+++ b/mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
@@ -69,6 +69,25 @@ func.func @sink_constant_step(%arg0: memref, %lb:
index, %ub: index) {
// -
+// Smoke test that parallel for doesn't crash when func dialect is not loaded.
+
+// CHECK-LABEL: llvm.func @without_func_dialect()
+llvm.func @without_func_dialect() {
+ %cst = arith.constant 0.0 : f32
+
+ %c0 = arith.constant 0 : index
+ %c22 = arith.constant 22 : index
+ %c1 = arith.constant 1 : index
+ %54 = memref.alloc() : memref<22xf32>
+ %alloc_4 = memref.alloc() : memref<22xf32>
+ scf.parallel (%arg0) = (%c0) to (%c22) step (%c1) {
+memref.store %cst, %alloc_4[%arg0] : memref<22xf32>
+ }
+ llvm.return
+}
+
+// -
+
// Check that for statically known inner loop bound block size is aligned and
// inner loop uses statically known loop trip counts.
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 842964b853d084d..963c52fd4191657 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1143,6 +1143,8 @@ void OpEmitter::genAttrNameGetters() {
const char *const getAttrName = R"(
assert(index < {0} && "invalid attribute index");
assert(name.getStringRef() == getOperationName() && "invalid operation
name");
+ assert(!name.getAttributeNames().empty() && "empty attribute names. Is a new
"
+ "op created without having initialized its dialect?");
return name.getAttributeNames()[index];
)";
method->body() << formatv(getAttrName, attributes.size());
>From eb09cc895d7d1c08f745df22345cd0fae5432c7a Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Mon, 20 Nov 2023 19:23:49 +0100
Subject: [PATCH 2/3] Declare dependentDialects in `Passes.td`
---
mlir/include/mlir/Dialect/Async/Passes.td | 1 +
mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp | 4
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/mlir/include/mlir/Dialect/Async/Passes.td
b/mlir/include/mlir/Dialect/Async/Passes.td
index c7ee4ba39aecdf0..f0ef83ca3fd4f1a 100644
--- a/mlir/include/mlir/Dialect/Async/Passes.td
+++ b/mlir/include/mlir/Dialect/Async/Passes.td
@@ -36,6 +36,7 @@ def AsyncParallelFor : Pass<"async-parallel-for", "ModuleOp">
{
let dependentDialects = [
"arith::ArithDialect",
"async::AsyncDialect",
+"func::FuncDialect",
"scf::SCFDialect"
];
}
diff --git a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
index 639bc7f9ec7f112..12a28c2e23b221a 100644
--- a/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
+++ b/mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
@@ -102,10 +102,6 @@ struct AsyncParallelForPass
: public impl::AsyncParallelForBase {
AsyncParallelForPass() = default;
- void getDependentDialects(DialectRegistry ®istry) const override {
-registry.insert();
- }
-
AsyncParallelForPass(bool asyncDispatch, int32_t numWorkerThreads,
int32_t minTaskSize) {
this->asyncDispatch = asyncDispatch;
>From 77ba982eba8f7511543e9e06864a15c839feece8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Mon, 20 Nov 2023 21:19:37 +0100
Subject: [PATCH 3/3] Update assertion
---
mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir | 2 +-
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp| 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/mlir/test/Dialect/Async/async-parallel-for-comp
[Lldb-commits] [mlir] [libc] [llvm] [openmp] [lldb] [libcxx] [clang] [lld] [flang] [clang-tools-extra] [compiler-rt] [mlir][async] Avoid crash when not using `func.func` (PR #72801)
https://github.com/rikhuijzer closed https://github.com/llvm/llvm-project/pull/72801 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [openmp] [lldb] [flang] [clang-tools-extra] [compiler-rt] [mlir] [llvm] [polly] [libc] [libcxxabi] [libcxx] [clang] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)
https://github.com/rikhuijzer updated
https://github.com/llvm/llvm-project/pull/76292
>From 0ff5a0ec09f7c26824bd90e6c7656222ee2448ae Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Sat, 23 Dec 2023 16:32:27 +0100
Subject: [PATCH 1/2] [mlir][vector] Fix invalid `LoadOp` indices being created
---
.../Conversion/VectorToSCF/VectorToSCF.cpp| 48 +--
.../Conversion/VectorToSCF/vector-to-scf.mlir | 37 ++
2 files changed, 71 insertions(+), 14 deletions(-)
diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index 2ee314e9fedfe3..13d2513a88804c 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -866,6 +866,31 @@ struct TransferOpConversion : public
VectorToSCFPattern {
this->setHasBoundedRewriteRecursion();
}
+ static void getMaskBufferLoadIndices(OpTy xferOp, Value castedMaskBuffer,
+ SmallVector &loadIndices,
+ Value iv) {
+assert(xferOp.getMask() && "Expected transfer op to have mask");
+
+// Add load indices from the previous iteration.
+// The mask buffer depends on the permutation map, which makes determining
+// the indices quite complex, so this is why we need to "look back" to the
+// previous iteration to find the right indices.
+Value maskBuffer = getMaskBuffer(xferOp);
+for (OpOperand &use : maskBuffer.getUses()) {
+ // If there is no previous load op, then the indices are empty.
+ if (auto loadOp = dyn_cast(use.getOwner())) {
+Operation::operand_range prevIndices = loadOp.getIndices();
+loadIndices.append(prevIndices.begin(), prevIndices.end());
+break;
+ }
+}
+
+// In case of broadcast: Use same indices to load from memref
+// as before.
+if (!xferOp.isBroadcastDim(0))
+ loadIndices.push_back(iv);
+ }
+
LogicalResult matchAndRewrite(OpTy xferOp,
PatternRewriter &rewriter) const override {
if (!xferOp->hasAttr(kPassLabel))
@@ -873,9 +898,9 @@ struct TransferOpConversion : public
VectorToSCFPattern {
// Find and cast data buffer. How the buffer can be found depends on OpTy.
ImplicitLocOpBuilder locB(xferOp.getLoc(), rewriter);
-auto dataBuffer = Strategy::getBuffer(xferOp);
+Value dataBuffer = Strategy::getBuffer(xferOp);
auto dataBufferType = dyn_cast(dataBuffer.getType());
-auto castedDataType = unpackOneDim(dataBufferType);
+FailureOr castedDataType = unpackOneDim(dataBufferType);
if (failed(castedDataType))
return failure();
@@ -885,8 +910,7 @@ struct TransferOpConversion : public
VectorToSCFPattern {
// If the xferOp has a mask: Find and cast mask buffer.
Value castedMaskBuffer;
if (xferOp.getMask()) {
- auto maskBuffer = getMaskBuffer(xferOp);
- auto maskBufferType = dyn_cast(maskBuffer.getType());
+ Value maskBuffer = getMaskBuffer(xferOp);
if (xferOp.isBroadcastDim(0) || xferOp.getMaskType().getRank() == 1) {
// Do not unpack a dimension of the mask, if:
// * To-be-unpacked transfer op dimension is a broadcast.
@@ -897,7 +921,8 @@ struct TransferOpConversion : public
VectorToSCFPattern {
} else {
// It's safe to assume the mask buffer can be unpacked if the data
// buffer was unpacked.
-auto castedMaskType = *unpackOneDim(maskBufferType);
+auto maskBufferType = dyn_cast(maskBuffer.getType());
+MemRefType castedMaskType = *unpackOneDim(maskBufferType);
castedMaskBuffer =
locB.create(castedMaskType, maskBuffer);
}
@@ -929,21 +954,16 @@ struct TransferOpConversion : public
VectorToSCFPattern {
// If old transfer op has a mask: Set mask on new transfer op.
// Special case: If the mask of the old transfer op is 1D and
-// the
-// unpacked dim is not a broadcast, no mask is
-// needed on the new transfer op.
+// the unpacked dim is not a broadcast, no mask is needed on
+// the new transfer op.
if (xferOp.getMask() && (xferOp.isBroadcastDim(0) ||
xferOp.getMaskType().getRank() > 1)) {
OpBuilder::InsertionGuard guard(b);
b.setInsertionPoint(newXfer); // Insert load before newXfer.
SmallVector loadIndices;
- Strategy::getBufferIndices(xferOp, loadIndices);
- // In case of broadcast: Use same indices to load from memref
- // as before.
- if (!xferOp.isBroadcastDim(0))
-loadIndices.push_back(iv);
-
+ getMaskBufferLoadIndices(xferOp, castedMaskBuffer,
+
[Lldb-commits] [clang-tools-extra] [polly] [libc] [flang] [openmp] [compiler-rt] [clang] [libcxxabi] [llvm] [mlir] [libcxx] [lldb] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)
https://github.com/rikhuijzer updated
https://github.com/llvm/llvm-project/pull/76292
>From 0ff5a0ec09f7c26824bd90e6c7656222ee2448ae Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Sat, 23 Dec 2023 16:32:27 +0100
Subject: [PATCH 1/3] [mlir][vector] Fix invalid `LoadOp` indices being created
---
.../Conversion/VectorToSCF/VectorToSCF.cpp| 48 +--
.../Conversion/VectorToSCF/vector-to-scf.mlir | 37 ++
2 files changed, 71 insertions(+), 14 deletions(-)
diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index 2ee314e9fedfe3..13d2513a88804c 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -866,6 +866,31 @@ struct TransferOpConversion : public
VectorToSCFPattern {
this->setHasBoundedRewriteRecursion();
}
+ static void getMaskBufferLoadIndices(OpTy xferOp, Value castedMaskBuffer,
+ SmallVector &loadIndices,
+ Value iv) {
+assert(xferOp.getMask() && "Expected transfer op to have mask");
+
+// Add load indices from the previous iteration.
+// The mask buffer depends on the permutation map, which makes determining
+// the indices quite complex, so this is why we need to "look back" to the
+// previous iteration to find the right indices.
+Value maskBuffer = getMaskBuffer(xferOp);
+for (OpOperand &use : maskBuffer.getUses()) {
+ // If there is no previous load op, then the indices are empty.
+ if (auto loadOp = dyn_cast(use.getOwner())) {
+Operation::operand_range prevIndices = loadOp.getIndices();
+loadIndices.append(prevIndices.begin(), prevIndices.end());
+break;
+ }
+}
+
+// In case of broadcast: Use same indices to load from memref
+// as before.
+if (!xferOp.isBroadcastDim(0))
+ loadIndices.push_back(iv);
+ }
+
LogicalResult matchAndRewrite(OpTy xferOp,
PatternRewriter &rewriter) const override {
if (!xferOp->hasAttr(kPassLabel))
@@ -873,9 +898,9 @@ struct TransferOpConversion : public
VectorToSCFPattern {
// Find and cast data buffer. How the buffer can be found depends on OpTy.
ImplicitLocOpBuilder locB(xferOp.getLoc(), rewriter);
-auto dataBuffer = Strategy::getBuffer(xferOp);
+Value dataBuffer = Strategy::getBuffer(xferOp);
auto dataBufferType = dyn_cast(dataBuffer.getType());
-auto castedDataType = unpackOneDim(dataBufferType);
+FailureOr castedDataType = unpackOneDim(dataBufferType);
if (failed(castedDataType))
return failure();
@@ -885,8 +910,7 @@ struct TransferOpConversion : public
VectorToSCFPattern {
// If the xferOp has a mask: Find and cast mask buffer.
Value castedMaskBuffer;
if (xferOp.getMask()) {
- auto maskBuffer = getMaskBuffer(xferOp);
- auto maskBufferType = dyn_cast(maskBuffer.getType());
+ Value maskBuffer = getMaskBuffer(xferOp);
if (xferOp.isBroadcastDim(0) || xferOp.getMaskType().getRank() == 1) {
// Do not unpack a dimension of the mask, if:
// * To-be-unpacked transfer op dimension is a broadcast.
@@ -897,7 +921,8 @@ struct TransferOpConversion : public
VectorToSCFPattern {
} else {
// It's safe to assume the mask buffer can be unpacked if the data
// buffer was unpacked.
-auto castedMaskType = *unpackOneDim(maskBufferType);
+auto maskBufferType = dyn_cast(maskBuffer.getType());
+MemRefType castedMaskType = *unpackOneDim(maskBufferType);
castedMaskBuffer =
locB.create(castedMaskType, maskBuffer);
}
@@ -929,21 +954,16 @@ struct TransferOpConversion : public
VectorToSCFPattern {
// If old transfer op has a mask: Set mask on new transfer op.
// Special case: If the mask of the old transfer op is 1D and
-// the
-// unpacked dim is not a broadcast, no mask is
-// needed on the new transfer op.
+// the unpacked dim is not a broadcast, no mask is needed on
+// the new transfer op.
if (xferOp.getMask() && (xferOp.isBroadcastDim(0) ||
xferOp.getMaskType().getRank() > 1)) {
OpBuilder::InsertionGuard guard(b);
b.setInsertionPoint(newXfer); // Insert load before newXfer.
SmallVector loadIndices;
- Strategy::getBufferIndices(xferOp, loadIndices);
- // In case of broadcast: Use same indices to load from memref
- // as before.
- if (!xferOp.isBroadcastDim(0))
-loadIndices.push_back(iv);
-
+ getMaskBufferLoadIndices(xferOp, castedMaskBuffer,
+
[Lldb-commits] [clang-tools-extra] [polly] [libc] [flang] [openmp] [compiler-rt] [clang] [libcxxabi] [llvm] [mlir] [libcxx] [lldb] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)
@@ -897,7 +921,8 @@ struct TransferOpConversion : public
VectorToSCFPattern {
} else {
// It's safe to assume the mask buffer can be unpacked if the data
// buffer was unpacked.
-auto castedMaskType = *unpackOneDim(maskBufferType);
+auto maskBufferType = dyn_cast(maskBuffer.getType());
rikhuijzer wrote:
Ah yes. You're right. Done in
[ec9d8d7](https://github.com/llvm/llvm-project/pull/76292/commits/ec9d8d75077b26c2efa92063ec659ba2dd89d8b7).
https://github.com/llvm/llvm-project/pull/76292
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [polly] [compiler-rt] [llvm] [libc] [libcxx] [clang-tools-extra] [openmp] [libcxxabi] [flang] [mlir] [clang] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)
https://github.com/rikhuijzer closed https://github.com/llvm/llvm-project/pull/76292 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [polly] [compiler-rt] [llvm] [libc] [libcxx] [clang-tools-extra] [openmp] [libcxxabi] [flang] [mlir] [clang] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)
rikhuijzer wrote: @joker-eph, thanks again for the review! https://github.com/llvm/llvm-project/pull/76292 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)
https://github.com/rikhuijzer updated
https://github.com/llvm/llvm-project/pull/68900
>From ddbde18e483d12485ba25c715e8a94480b9d6dcf Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Thu, 12 Oct 2023 16:55:22 +0200
Subject: [PATCH 1/3] [mlir][arith] Fix canon pattern for large ints in chained
arith
---
mlir/lib/Dialect/Arith/IR/ArithOps.cpp| 25 +++
mlir/test/Dialect/Arith/canonicalize.mlir | 10 +
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 0ecc288f3b07701..25578b1c52f331b 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -39,26 +39,35 @@ using namespace mlir::arith;
static IntegerAttr
applyToIntegerAttrs(PatternRewriter &builder, Value res, Attribute lhs,
Attribute rhs,
-function_ref binFn) {
- return builder.getIntegerAttr(res.getType(),
-binFn(llvm::cast(lhs).getInt(),
- llvm::cast(rhs).getInt()));
+function_ref binFn) {
+ auto lhsVal = llvm::cast(lhs).getValue();
+ auto rhsVal = llvm::cast(rhs).getValue();
+ auto value = binFn(lhsVal, rhsVal);
+ return IntegerAttr::get(res.getType(), value);
}
static IntegerAttr addIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
- return applyToIntegerAttrs(builder, res, lhs, rhs, std::plus());
+ auto binFn = [](APInt a, APInt& b) -> APInt {
+return std::move(a) + b;
+ };
+ return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
static IntegerAttr subIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
- return applyToIntegerAttrs(builder, res, lhs, rhs, std::minus());
+ auto binFn = [](APInt a, APInt& b) -> APInt {
+return std::move(a) - b;
+ };
+ return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
static IntegerAttr mulIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
- return applyToIntegerAttrs(builder, res, lhs, rhs,
- std::multiplies());
+ auto binFn = [](APInt a, APInt& b) -> APInt {
+return std::move(a) * b;
+ };
+ return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
/// Invert an integer comparison predicate.
diff --git a/mlir/test/Dialect/Arith/canonicalize.mlir
b/mlir/test/Dialect/Arith/canonicalize.mlir
index 1b0547c9e8f804a..b18f5cfcb3f9a12 100644
--- a/mlir/test/Dialect/Arith/canonicalize.mlir
+++ b/mlir/test/Dialect/Arith/canonicalize.mlir
@@ -985,6 +985,16 @@ func.func @tripleMulIMulII32(%arg0: i32) -> i32 {
return %mul2 : i32
}
+// CHECK-LABEL: @tripleMulLargeInt
+// CHECK: return
+func.func @tripleMulLargeInt(%arg0: i256) -> i256 {
+ %0 = arith.constant
3618502788666131213697322783095070105623107215331596699973092056135872020481 :
i256
+ %c5 = arith.constant 5 : i256
+ %mul1 = arith.muli %arg0, %0 : i256
+ %mul2 = arith.muli %mul1, %c5 : i256
+ return %mul2 : i256
+}
+
// CHECK-LABEL: @addiMuliToSubiRhsI32
// CHECK-SAME: (%[[ARG0:.+]]: i32, %[[ARG1:.+]]: i32)
// CHECK: %[[SUB:.+]] = arith.subi %[[ARG0]], %[[ARG1]] : i32
>From c0f3efe78fa6e71d1acc4d38f526ca2ec194ddf8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Fri, 13 Oct 2023 10:14:16 +0200
Subject: [PATCH 2/3] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Markus Böck
---
mlir/lib/Dialect/Arith/IR/ArithOps.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 25578b1c52f331b..b749af256e7 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -39,7 +39,7 @@ using namespace mlir::arith;
static IntegerAttr
applyToIntegerAttrs(PatternRewriter &builder, Value res, Attribute lhs,
Attribute rhs,
-function_ref binFn) {
+function_ref binFn) {
auto lhsVal = llvm::cast(lhs).getValue();
auto rhsVal = llvm::cast(rhs).getValue();
auto value = binFn(lhsVal, rhsVal);
@@ -49,7 +49,7 @@ applyToIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs,
static IntegerAttr addIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
auto binFn = [](APInt a, APInt& b) -> APInt {
-return std::move(a) + b;
+return a + b;
};
return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
>From 30e1ce11d567452dcd7481e999109d1f25164065 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Fri, 13 Oct 2023 10:49:20 +0200
Subject: [PATCH 3/3] Use `const`s and check result o
[Lldb-commits] [lldb] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)
https://github.com/rikhuijzer updated
https://github.com/llvm/llvm-project/pull/68900
>From ddbde18e483d12485ba25c715e8a94480b9d6dcf Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Thu, 12 Oct 2023 16:55:22 +0200
Subject: [PATCH 1/4] [mlir][arith] Fix canon pattern for large ints in chained
arith
---
mlir/lib/Dialect/Arith/IR/ArithOps.cpp| 25 +++
mlir/test/Dialect/Arith/canonicalize.mlir | 10 +
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 0ecc288f3b07701..25578b1c52f331b 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -39,26 +39,35 @@ using namespace mlir::arith;
static IntegerAttr
applyToIntegerAttrs(PatternRewriter &builder, Value res, Attribute lhs,
Attribute rhs,
-function_ref binFn) {
- return builder.getIntegerAttr(res.getType(),
-binFn(llvm::cast(lhs).getInt(),
- llvm::cast(rhs).getInt()));
+function_ref binFn) {
+ auto lhsVal = llvm::cast(lhs).getValue();
+ auto rhsVal = llvm::cast(rhs).getValue();
+ auto value = binFn(lhsVal, rhsVal);
+ return IntegerAttr::get(res.getType(), value);
}
static IntegerAttr addIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
- return applyToIntegerAttrs(builder, res, lhs, rhs, std::plus());
+ auto binFn = [](APInt a, APInt& b) -> APInt {
+return std::move(a) + b;
+ };
+ return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
static IntegerAttr subIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
- return applyToIntegerAttrs(builder, res, lhs, rhs, std::minus());
+ auto binFn = [](APInt a, APInt& b) -> APInt {
+return std::move(a) - b;
+ };
+ return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
static IntegerAttr mulIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
- return applyToIntegerAttrs(builder, res, lhs, rhs,
- std::multiplies());
+ auto binFn = [](APInt a, APInt& b) -> APInt {
+return std::move(a) * b;
+ };
+ return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
/// Invert an integer comparison predicate.
diff --git a/mlir/test/Dialect/Arith/canonicalize.mlir
b/mlir/test/Dialect/Arith/canonicalize.mlir
index 1b0547c9e8f804a..b18f5cfcb3f9a12 100644
--- a/mlir/test/Dialect/Arith/canonicalize.mlir
+++ b/mlir/test/Dialect/Arith/canonicalize.mlir
@@ -985,6 +985,16 @@ func.func @tripleMulIMulII32(%arg0: i32) -> i32 {
return %mul2 : i32
}
+// CHECK-LABEL: @tripleMulLargeInt
+// CHECK: return
+func.func @tripleMulLargeInt(%arg0: i256) -> i256 {
+ %0 = arith.constant
3618502788666131213697322783095070105623107215331596699973092056135872020481 :
i256
+ %c5 = arith.constant 5 : i256
+ %mul1 = arith.muli %arg0, %0 : i256
+ %mul2 = arith.muli %mul1, %c5 : i256
+ return %mul2 : i256
+}
+
// CHECK-LABEL: @addiMuliToSubiRhsI32
// CHECK-SAME: (%[[ARG0:.+]]: i32, %[[ARG1:.+]]: i32)
// CHECK: %[[SUB:.+]] = arith.subi %[[ARG0]], %[[ARG1]] : i32
>From c0f3efe78fa6e71d1acc4d38f526ca2ec194ddf8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Fri, 13 Oct 2023 10:14:16 +0200
Subject: [PATCH 2/4] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Markus Böck
---
mlir/lib/Dialect/Arith/IR/ArithOps.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 25578b1c52f331b..b749af256e7 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -39,7 +39,7 @@ using namespace mlir::arith;
static IntegerAttr
applyToIntegerAttrs(PatternRewriter &builder, Value res, Attribute lhs,
Attribute rhs,
-function_ref binFn) {
+function_ref binFn) {
auto lhsVal = llvm::cast(lhs).getValue();
auto rhsVal = llvm::cast(rhs).getValue();
auto value = binFn(lhsVal, rhsVal);
@@ -49,7 +49,7 @@ applyToIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs,
static IntegerAttr addIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
auto binFn = [](APInt a, APInt& b) -> APInt {
-return std::move(a) + b;
+return a + b;
};
return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
}
>From 30e1ce11d567452dcd7481e999109d1f25164065 Mon Sep 17 00:00:00 2001
From: Rik Huijzer
Date: Fri, 13 Oct 2023 10:49:20 +0200
Subject: [PATCH 3/4] Use `const`s and check result o
[Lldb-commits] [lldb] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)
@@ -39,26 +39,29 @@ using namespace mlir::arith;
static IntegerAttr
applyToIntegerAttrs(PatternRewriter &builder, Value res, Attribute lhs,
Attribute rhs,
-function_ref binFn) {
- return builder.getIntegerAttr(res.getType(),
-binFn(llvm::cast(lhs).getInt(),
- llvm::cast(rhs).getInt()));
+function_ref binFn) {
+ APInt lhsVal = llvm::cast(lhs).getValue();
+ APInt rhsVal = llvm::cast(rhs).getValue();
+ APInt value = binFn(lhsVal, rhsVal);
+ return IntegerAttr::get(res.getType(), value);
}
static IntegerAttr addIntegerAttrs(PatternRewriter &builder, Value res,
Attribute lhs, Attribute rhs) {
- return applyToIntegerAttrs(builder, res, lhs, rhs, std::plus());
+ auto binFn = [](const APInt &a, const APInt &b) -> APInt { return a + b; };
+ return applyToIntegerAttrs(builder, res, lhs, rhs, binFn);
rikhuijzer wrote:
Well spotted 👍
https://github.com/llvm/llvm-project/pull/68900
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)
https://github.com/rikhuijzer closed https://github.com/llvm/llvm-project/pull/68900 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [mlir] Verify TestBuiltinAttributeInterfaces eltype (PR #69878)
https://github.com/rikhuijzer closed https://github.com/llvm/llvm-project/pull/69878 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
