[mlir] [llvm] [flang] [clang-tools-extra] [compiler-rt] [clang] [mlir][memref] Detect negative `offset` or `size` for `subview` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/72059

>From 8f90ae9113229faf5ab4b859e5c70f23853f4db8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sat, 11 Nov 2023 20:43:31 +0100
Subject: [PATCH 1/8] [mlir][memref] Verify subview offsets and sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 12 +++-
 mlir/test/Dialect/MemRef/invalid.mlir| 16 
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 215a8f5e7d18be0..dc3e04be83fc1c9 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2842,8 +2842,18 @@ static LogicalResult 
produceSubViewErrorMsg(SliceVerificationResult result,
   llvm_unreachable("unexpected subview verification result");
 }
 
-/// Verifier for SubViewOp.
 LogicalResult SubViewOp::verify() {
+  for (int64_t offset : getStaticOffsets()) {
+if (offset < 0 && !ShapedType::isDynamic(offset))
+  return emitError("expected subview offsets to be non-negative, but got ")
+ << offset;
+  }
+  for (int64_t size : getStaticSizes()) {
+if (size < 1 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be positive, but got ")
+ << size;
+  }
+
   MemRefType baseType = getSourceType();
   MemRefType subViewType = getType();
 
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index cb5977e302a993f..17f5fbea3bda17a 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -611,6 +611,22 @@ func.func @invalid_view(%arg0 : index, %arg1 : index, 
%arg2 : index) {
 
 // -
 
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview offsets to be non-negative, but got 
-1}}
+  %0 = memref.subview %input[-1, 256] [2, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
+  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
 func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
   %0 = memref.alloc() : memref<8x16x4xf32>
   // expected-error@+1 {{expected mixed offsets rank to match mixed sizes rank 
(2 vs 3) so the rank of the result type is well-formed}}

>From 32adf97c8d19e29e4673c772bd190c0eb0b413e9 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sun, 12 Nov 2023 09:10:00 +0100
Subject: [PATCH 2/8] Allow zero sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 5 +++--
 mlir/test/Dialect/MemRef/invalid.mlir| 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index dc3e04be83fc1c9..a24467a321a70c5 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2849,8 +2849,8 @@ LogicalResult SubViewOp::verify() {
  << offset;
   }
   for (int64_t size : getStaticSizes()) {
-if (size < 1 && !ShapedType::isDynamic(size))
-  return emitError("expected subview sizes to be positive, but got ")
+if (size < 0 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be non-negative, but got ")
  << size;
   }
 
@@ -3103,6 +3103,7 @@ struct SubViewReturnTypeCanonicalizer {
   MemRefType operator()(SubViewOp op, ArrayRef mixedOffsets,
 ArrayRef mixedSizes,
 ArrayRef mixedStrides) {
+
 // Infer a memref type without taking into account any rank reductions.
 MemRefType nonReducedType = cast(SubViewOp::inferResultType(
 op.getSourceType(), mixedOffsets, mixedSizes, mixedStrides));
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index 17f5fbea3bda17a..38c0bcc3f2491c2 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -620,8 +620,8 @@ func.func @invalid_subview(%input: memref<4x1024xf32>) -> 
memref<2x256xf32, stri
 // -
 
 func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
-  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
-  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  // expected-error@+1 {{expected subview sizes to be non-negative, but g

[mlir] [llvm] [clang-tools-extra] [compiler-rt] [clang] [mlir][tensor] Document `dest` operand (PR #71726)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/71726

>From 3f34a440abef46c5c6280fdcdf0c29e05dda4565 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Wed, 8 Nov 2023 20:10:41 +0100
Subject: [PATCH 1/4] [mlir][tensor] Document `dest` operand

---
 .../mlir/Dialect/Tensor/IR/TensorBase.td  | 34 ++-
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td 
b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
index 1231c0a67bc305f..768edec27a755a6 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
+++ b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
@@ -18,31 +18,33 @@ def Tensor_Dialect : Dialect {
   let description = [{
 The `tensor` dialect is intended to hold core tensor creation and
 manipulation ops, which are not strongly associated with any particular
-other dialect or domain abstraction. The primary smoke test of this is ops
-that make sense for any tensor element type.
-
-We leave it to other dialects to hold the vast swath of possible
-computations one might want to do on a tensor.
-
-The `tensor` type is (for better or for worse) used to represent all kinds
-of things, and supports an open-ended set of element types. Examples:
+other dialect or domain abstraction. The primary inclusion criteria for ops
+in this dialect is that they make sense for any tensor element type. When
+this is not the case, the op is left to live in other dialects. Examples of
+element types that could be supported by the `tensor` dialect include:
 
 - representing large, dense aggregations of primitive types, suitable for
   high-performance numerical computing.
-- representing shapes in the `shape` dialect, which consist of small
-  1D tensors of `index` data type.
+- representing shapes in the `shape` dialect, which consist of small 1D
+  tensors of `index` data type.
 - representing aggregations of strings or “variant” types.
-- representing large, sparse aggregations of primitive types, suitable
-  for high-performance numerical computing.
+- representing large, sparse aggregations of primitive types, suitable for
+  high-performance numerical computing.
 
-Thus, for the `tensor` dialect, we prefer for now to constrain the
-scope as much as possible. The expectation is that at some point
+Because of this broad element type support, we prefer for now to keep the
+`tensor` dialect as small as possible. The expectation is that at some 
point
 in the future, the `tensor` dialect’s scope may be broadened through a
 careful discussion of the tradeoffs.
 
-The `tensor` type is actually a builtin type (it lives in the builtin
-dialect), and does not live in this dialect.
+One exception to the above is the `tensor` type itself, which is actually a
+builtin type (it lives in the builtin dialect), and does not live in this
+dialect.
 
+Finally, many ops in the the dialect use the `dest` operand. This is an
+operand that is used to encode information for bufferization via the
+`DestinationStyleOpInterface`, see the [Destination Passing Style](
+https://mlir.llvm.org/docs/Bufferization/#destination-passing-style)
+documentation for more information.
   }];
 
   let hasCanonicalizer = 1;

>From a72c2b5b5a2d0bad4203e4085255c95829dee7dd Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Thu, 9 Nov 2023 17:08:59 +0100
Subject: [PATCH 2/4] Try to implement suggestion

---
 mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td 
b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
index 768edec27a755a6..9d5c7cdaea0465a 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
+++ b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
@@ -36,15 +36,14 @@ def Tensor_Dialect : Dialect {
 in the future, the `tensor` dialect’s scope may be broadened through a
 careful discussion of the tradeoffs.
 
-One exception to the above is the `tensor` type itself, which is actually a
-builtin type (it lives in the builtin dialect), and does not live in this
-dialect.
-
-Finally, many ops in the the dialect use the `dest` operand. This is an
-operand that is used to encode information for bufferization via the
-`DestinationStyleOpInterface`, see the [Destination Passing Style](
+On the `tensor` type itself, note that it is actually a builtin type (it
+lives in the builtin dialect), and does not live in this dialect. 
Furthermore,
+a `tensor` is an immutable object. For example, this means that the `dest`
+operand used by some ops in this dialect does not mean that the `tensor` is
+mutated in place, but rather that the operand can be used as bufferization
+hint. For more information, see the [

[flang] [clang] [clang-tools-extra] [mlir] [compiler-rt] [llvm] [mlir][memref] Detect negative `offset` or `size` for `subview` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/72059

>From 8f90ae9113229faf5ab4b859e5c70f23853f4db8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sat, 11 Nov 2023 20:43:31 +0100
Subject: [PATCH 1/9] [mlir][memref] Verify subview offsets and sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 12 +++-
 mlir/test/Dialect/MemRef/invalid.mlir| 16 
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 215a8f5e7d18be0..dc3e04be83fc1c9 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2842,8 +2842,18 @@ static LogicalResult 
produceSubViewErrorMsg(SliceVerificationResult result,
   llvm_unreachable("unexpected subview verification result");
 }
 
-/// Verifier for SubViewOp.
 LogicalResult SubViewOp::verify() {
+  for (int64_t offset : getStaticOffsets()) {
+if (offset < 0 && !ShapedType::isDynamic(offset))
+  return emitError("expected subview offsets to be non-negative, but got ")
+ << offset;
+  }
+  for (int64_t size : getStaticSizes()) {
+if (size < 1 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be positive, but got ")
+ << size;
+  }
+
   MemRefType baseType = getSourceType();
   MemRefType subViewType = getType();
 
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index cb5977e302a993f..17f5fbea3bda17a 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -611,6 +611,22 @@ func.func @invalid_view(%arg0 : index, %arg1 : index, 
%arg2 : index) {
 
 // -
 
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview offsets to be non-negative, but got 
-1}}
+  %0 = memref.subview %input[-1, 256] [2, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
+  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
 func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
   %0 = memref.alloc() : memref<8x16x4xf32>
   // expected-error@+1 {{expected mixed offsets rank to match mixed sizes rank 
(2 vs 3) so the rank of the result type is well-formed}}

>From 32adf97c8d19e29e4673c772bd190c0eb0b413e9 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sun, 12 Nov 2023 09:10:00 +0100
Subject: [PATCH 2/9] Allow zero sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 5 +++--
 mlir/test/Dialect/MemRef/invalid.mlir| 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index dc3e04be83fc1c9..a24467a321a70c5 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2849,8 +2849,8 @@ LogicalResult SubViewOp::verify() {
  << offset;
   }
   for (int64_t size : getStaticSizes()) {
-if (size < 1 && !ShapedType::isDynamic(size))
-  return emitError("expected subview sizes to be positive, but got ")
+if (size < 0 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be non-negative, but got ")
  << size;
   }
 
@@ -3103,6 +3103,7 @@ struct SubViewReturnTypeCanonicalizer {
   MemRefType operator()(SubViewOp op, ArrayRef mixedOffsets,
 ArrayRef mixedSizes,
 ArrayRef mixedStrides) {
+
 // Infer a memref type without taking into account any rank reductions.
 MemRefType nonReducedType = cast(SubViewOp::inferResultType(
 op.getSourceType(), mixedOffsets, mixedSizes, mixedStrides));
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index 17f5fbea3bda17a..38c0bcc3f2491c2 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -620,8 +620,8 @@ func.func @invalid_subview(%input: memref<4x1024xf32>) -> 
memref<2x256xf32, stri
 // -
 
 func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
-  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
-  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  // expected-error@+1 {{expected subview sizes to be non-negative, but g

[flang] [clang] [clang-tools-extra] [mlir] [compiler-rt] [llvm] [mlir][memref] Detect negative `offset` or `size` for `subview` (PR #72059)

[Rik Huijzer via cfe-commits]

@@ -2621,6 +2621,15 @@ Type SubViewOp::inferResultType(MemRefType 
sourceMemRefType,
   dispatchIndexOpFoldResults(offsets, dynamicOffsets, staticOffsets);
   dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
   dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
+
+  for (int64_t offset : staticOffsets) {

rikhuijzer wrote:

Because loops are relatively expensive you mean? Is this comment outdated when 
the logic moves to the interface and is specified as an invariant?

https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [llvm] [mlir] [clang] [flang] [clang-tools-extra] [mlir][memref] Detect negative `offset` or `size` for `subview` (PR #72059)

[Rik Huijzer via cfe-commits]

@@ -2842,8 +2851,18 @@ static LogicalResult 
produceSubViewErrorMsg(SliceVerificationResult result,
   llvm_unreachable("unexpected subview verification result");
 }
 
-/// Verifier for SubViewOp.
 LogicalResult SubViewOp::verify() {
+  for (int64_t offset : getStaticOffsets()) {

rikhuijzer wrote:

Yes that makes a lot of sense IMO. I find it quite impressive from the MLIR 
developers that adding the check (e4e40f2) causes none of the tests to fail! 
Hopefully nobody downstream depends on this logic even though it shouldn't work 
anyway.

https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [llvm] [mlir] [clang] [flang] [clang-tools-extra] [mlir][memref] Detect negative `offset` or `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [llvm] [mlir] [clang] [flang] [clang-tools-extra] [mlir] Verify non-negative `offset` or `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[flang] [clang] [clang-tools-extra] [mlir] [compiler-rt] [llvm] [mlir] Verify non-negative `offset` or `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[flang] [clang] [clang-tools-extra] [mlir] [compiler-rt] [llvm] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [clang] [clang-tools-extra] [llvm] [mlir] [mlir][tensor] Document `dest` operand (PR #71726)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/71726

>From 3f34a440abef46c5c6280fdcdf0c29e05dda4565 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Wed, 8 Nov 2023 20:10:41 +0100
Subject: [PATCH 1/5] [mlir][tensor] Document `dest` operand

---
 .../mlir/Dialect/Tensor/IR/TensorBase.td  | 34 ++-
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td 
b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
index 1231c0a67bc305f..768edec27a755a6 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
+++ b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
@@ -18,31 +18,33 @@ def Tensor_Dialect : Dialect {
   let description = [{
 The `tensor` dialect is intended to hold core tensor creation and
 manipulation ops, which are not strongly associated with any particular
-other dialect or domain abstraction. The primary smoke test of this is ops
-that make sense for any tensor element type.
-
-We leave it to other dialects to hold the vast swath of possible
-computations one might want to do on a tensor.
-
-The `tensor` type is (for better or for worse) used to represent all kinds
-of things, and supports an open-ended set of element types. Examples:
+other dialect or domain abstraction. The primary inclusion criteria for ops
+in this dialect is that they make sense for any tensor element type. When
+this is not the case, the op is left to live in other dialects. Examples of
+element types that could be supported by the `tensor` dialect include:
 
 - representing large, dense aggregations of primitive types, suitable for
   high-performance numerical computing.
-- representing shapes in the `shape` dialect, which consist of small
-  1D tensors of `index` data type.
+- representing shapes in the `shape` dialect, which consist of small 1D
+  tensors of `index` data type.
 - representing aggregations of strings or “variant” types.
-- representing large, sparse aggregations of primitive types, suitable
-  for high-performance numerical computing.
+- representing large, sparse aggregations of primitive types, suitable for
+  high-performance numerical computing.
 
-Thus, for the `tensor` dialect, we prefer for now to constrain the
-scope as much as possible. The expectation is that at some point
+Because of this broad element type support, we prefer for now to keep the
+`tensor` dialect as small as possible. The expectation is that at some 
point
 in the future, the `tensor` dialect’s scope may be broadened through a
 careful discussion of the tradeoffs.
 
-The `tensor` type is actually a builtin type (it lives in the builtin
-dialect), and does not live in this dialect.
+One exception to the above is the `tensor` type itself, which is actually a
+builtin type (it lives in the builtin dialect), and does not live in this
+dialect.
 
+Finally, many ops in the the dialect use the `dest` operand. This is an
+operand that is used to encode information for bufferization via the
+`DestinationStyleOpInterface`, see the [Destination Passing Style](
+https://mlir.llvm.org/docs/Bufferization/#destination-passing-style)
+documentation for more information.
   }];
 
   let hasCanonicalizer = 1;

>From a72c2b5b5a2d0bad4203e4085255c95829dee7dd Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Thu, 9 Nov 2023 17:08:59 +0100
Subject: [PATCH 2/5] Try to implement suggestion

---
 mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td 
b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
index 768edec27a755a6..9d5c7cdaea0465a 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
+++ b/mlir/include/mlir/Dialect/Tensor/IR/TensorBase.td
@@ -36,15 +36,14 @@ def Tensor_Dialect : Dialect {
 in the future, the `tensor` dialect’s scope may be broadened through a
 careful discussion of the tradeoffs.
 
-One exception to the above is the `tensor` type itself, which is actually a
-builtin type (it lives in the builtin dialect), and does not live in this
-dialect.
-
-Finally, many ops in the the dialect use the `dest` operand. This is an
-operand that is used to encode information for bufferization via the
-`DestinationStyleOpInterface`, see the [Destination Passing Style](
+On the `tensor` type itself, note that it is actually a builtin type (it
+lives in the builtin dialect), and does not live in this dialect. 
Furthermore,
+a `tensor` is an immutable object. For example, this means that the `dest`
+operand used by some ops in this dialect does not mean that the `tensor` is
+mutated in place, but rather that the operand can be used as bufferization
+hint. For more information, see the [

[flang] [mlir] [clang] [compiler-rt] [llvm] [clang-tools-extra] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/72059

>From 8f90ae9113229faf5ab4b859e5c70f23853f4db8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sat, 11 Nov 2023 20:43:31 +0100
Subject: [PATCH 01/10] [mlir][memref] Verify subview offsets and sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 12 +++-
 mlir/test/Dialect/MemRef/invalid.mlir| 16 
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 215a8f5e7d18be0..dc3e04be83fc1c9 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2842,8 +2842,18 @@ static LogicalResult 
produceSubViewErrorMsg(SliceVerificationResult result,
   llvm_unreachable("unexpected subview verification result");
 }
 
-/// Verifier for SubViewOp.
 LogicalResult SubViewOp::verify() {
+  for (int64_t offset : getStaticOffsets()) {
+if (offset < 0 && !ShapedType::isDynamic(offset))
+  return emitError("expected subview offsets to be non-negative, but got ")
+ << offset;
+  }
+  for (int64_t size : getStaticSizes()) {
+if (size < 1 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be positive, but got ")
+ << size;
+  }
+
   MemRefType baseType = getSourceType();
   MemRefType subViewType = getType();
 
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index cb5977e302a993f..17f5fbea3bda17a 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -611,6 +611,22 @@ func.func @invalid_view(%arg0 : index, %arg1 : index, 
%arg2 : index) {
 
 // -
 
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview offsets to be non-negative, but got 
-1}}
+  %0 = memref.subview %input[-1, 256] [2, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
+  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
 func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
   %0 = memref.alloc() : memref<8x16x4xf32>
   // expected-error@+1 {{expected mixed offsets rank to match mixed sizes rank 
(2 vs 3) so the rank of the result type is well-formed}}

>From 32adf97c8d19e29e4673c772bd190c0eb0b413e9 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sun, 12 Nov 2023 09:10:00 +0100
Subject: [PATCH 02/10] Allow zero sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 5 +++--
 mlir/test/Dialect/MemRef/invalid.mlir| 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index dc3e04be83fc1c9..a24467a321a70c5 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2849,8 +2849,8 @@ LogicalResult SubViewOp::verify() {
  << offset;
   }
   for (int64_t size : getStaticSizes()) {
-if (size < 1 && !ShapedType::isDynamic(size))
-  return emitError("expected subview sizes to be positive, but got ")
+if (size < 0 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be non-negative, but got ")
  << size;
   }
 
@@ -3103,6 +3103,7 @@ struct SubViewReturnTypeCanonicalizer {
   MemRefType operator()(SubViewOp op, ArrayRef mixedOffsets,
 ArrayRef mixedSizes,
 ArrayRef mixedStrides) {
+
 // Infer a memref type without taking into account any rank reductions.
 MemRefType nonReducedType = cast(SubViewOp::inferResultType(
 op.getSourceType(), mixedOffsets, mixedSizes, mixedStrides));
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index 17f5fbea3bda17a..38c0bcc3f2491c2 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -620,8 +620,8 @@ func.func @invalid_subview(%input: memref<4x1024xf32>) -> 
memref<2x256xf32, stri
 // -
 
 func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
-  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
-  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  // expected-error@+1 {{expected subview sizes to be non-negative, b

[flang] [mlir] [clang] [compiler-rt] [llvm] [clang-tools-extra] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/72059

>From 8f90ae9113229faf5ab4b859e5c70f23853f4db8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sat, 11 Nov 2023 20:43:31 +0100
Subject: [PATCH 01/11] [mlir][memref] Verify subview offsets and sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 12 +++-
 mlir/test/Dialect/MemRef/invalid.mlir| 16 
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 215a8f5e7d18be0..dc3e04be83fc1c9 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2842,8 +2842,18 @@ static LogicalResult 
produceSubViewErrorMsg(SliceVerificationResult result,
   llvm_unreachable("unexpected subview verification result");
 }
 
-/// Verifier for SubViewOp.
 LogicalResult SubViewOp::verify() {
+  for (int64_t offset : getStaticOffsets()) {
+if (offset < 0 && !ShapedType::isDynamic(offset))
+  return emitError("expected subview offsets to be non-negative, but got ")
+ << offset;
+  }
+  for (int64_t size : getStaticSizes()) {
+if (size < 1 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be positive, but got ")
+ << size;
+  }
+
   MemRefType baseType = getSourceType();
   MemRefType subViewType = getType();
 
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index cb5977e302a993f..17f5fbea3bda17a 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -611,6 +611,22 @@ func.func @invalid_view(%arg0 : index, %arg1 : index, 
%arg2 : index) {
 
 // -
 
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview offsets to be non-negative, but got 
-1}}
+  %0 = memref.subview %input[-1, 256] [2, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
+  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
 func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
   %0 = memref.alloc() : memref<8x16x4xf32>
   // expected-error@+1 {{expected mixed offsets rank to match mixed sizes rank 
(2 vs 3) so the rank of the result type is well-formed}}

>From 32adf97c8d19e29e4673c772bd190c0eb0b413e9 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sun, 12 Nov 2023 09:10:00 +0100
Subject: [PATCH 02/11] Allow zero sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 5 +++--
 mlir/test/Dialect/MemRef/invalid.mlir| 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index dc3e04be83fc1c9..a24467a321a70c5 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2849,8 +2849,8 @@ LogicalResult SubViewOp::verify() {
  << offset;
   }
   for (int64_t size : getStaticSizes()) {
-if (size < 1 && !ShapedType::isDynamic(size))
-  return emitError("expected subview sizes to be positive, but got ")
+if (size < 0 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be non-negative, but got ")
  << size;
   }
 
@@ -3103,6 +3103,7 @@ struct SubViewReturnTypeCanonicalizer {
   MemRefType operator()(SubViewOp op, ArrayRef mixedOffsets,
 ArrayRef mixedSizes,
 ArrayRef mixedStrides) {
+
 // Infer a memref type without taking into account any rank reductions.
 MemRefType nonReducedType = cast(SubViewOp::inferResultType(
 op.getSourceType(), mixedOffsets, mixedSizes, mixedStrides));
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index 17f5fbea3bda17a..38c0bcc3f2491c2 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -620,8 +620,8 @@ func.func @invalid_subview(%input: memref<4x1024xf32>) -> 
memref<2x256xf32, stri
 // -
 
 func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
-  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
-  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  // expected-error@+1 {{expected subview sizes to be non-negative, b

[compiler-rt] [clang-tools-extra] [clang] [llvm] [mlir] [mlir][tensor] Document `dest` operand (PR #71726)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/71726
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[flang] [mlir] [compiler-rt] [clang-tools-extra] [clang] [libcxx] [llvm] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/74200

>From 22928e7e5da508d8d9dc8d4b7e54f84cccadef06 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 20 Nov 2023 09:02:41 +0100
Subject: [PATCH 1/6] [mlir][tensor] Fix canon via `hasNegativeDimension`

---
 mlir/include/mlir/Dialect/Tensor/IR/Tensor.h |  6 ++
 mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 15 +++
 mlir/test/Dialect/Tensor/canonicalize.mlir   | 10 ++
 3 files changed, 31 insertions(+)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h 
b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
index 06642adda42b3..0d027057b3a95 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
+++ b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
@@ -150,6 +150,12 @@ LogicalResult getOrCreateDestinations(OpBuilder &b, 
Location loc, Operation *op,
 /// Tests if types are the same when ignoring encoding on ranked tensors.
 bool isSameTypeWithoutEncoding(Type tp1, Type tp2);
 
+/// Helper function to check whether the dimensions are non-negative. This
+/// check also occurs in the verifier, but we need it at later stages too
+/// because the verifier ignores dynamic dimensions, but later stages might
+/// have constant folded those to (negative) constants.
+bool hasNegativeDimension(SmallVector shape);
+
 /// Function to control the folding of constant and extract slice.
 using ControlConstantExtractSliceFusionFn = 
std::function;
 
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp 
b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index e469815496e18..3297ef673ca2e 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -125,6 +125,12 @@ bool tensor::isSameTypeWithoutEncoding(Type tp1, Type tp2) 
{
   return tp1 == tp2; // default implementation
 }
 
+bool tensor::hasNegativeDimension(SmallVector shape) {
+  return llvm::any_of(shape, [](int64_t dim) {
+return !ShapedType::isDynamic(dim) && dim < 0;
+  });
+}
+
 /// Compute the dropped dimensions of a rank-reducing tensor.extract_slice op 
or
 /// rank-extending tensor.insert_slice op.
 static llvm::SmallBitVector getDroppedDims(ArrayRef reducedShape,
@@ -1801,6 +1807,10 @@ RankedTensorType 
ExtractSliceOp::inferCanonicalRankReducedResultType(
   dispatchIndexOpFoldResults(offsets, dynamicOffsets, staticOffsets);
   dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
   dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
+  if (hasNegativeDimension(staticOffsets))
+return {};
+  if (hasNegativeDimension(staticSizes))
+return {};
   return ExtractSliceOp::inferCanonicalRankReducedResultType(
   desiredResultRank, sourceRankedTensorType, staticOffsets, staticSizes,
   staticStrides);
@@ -2370,6 +2380,8 @@ class InsertSliceOpConstantArgumentFolder final
 auto sourceType = ExtractSliceOp::inferCanonicalRankReducedResultType(
 insertSliceOp.getSourceType().getRank(), insertSliceOp.getDestType(),
 mixedOffsets, mixedSizes, mixedStrides);
+if (!sourceType)
+  return failure();
 Value toInsert = insertSliceOp.getSource();
 if (sourceType != insertSliceOp.getSourceType()) {
   OpBuilder::InsertionGuard g(rewriter);
@@ -2500,6 +2512,8 @@ struct InsertSliceOpSourceCastInserter final
   getConstantIntValue(insertSliceOp.getMixedSizes()[i]))
 newSrcShape[i] = *constInt;
 }
+// if (hasNegativeDimension(newSrcShape))
+//  return failure();
 
 RankedTensorType newSrcType =
 RankedTensorType::get(newSrcShape, srcType.getElementType());
@@ -2521,6 +2535,7 @@ struct InsertSliceOpSourceCastInserter final
   rewriter.setInsertionPoint(insertSliceOp->getParentOp());
 Value cast = rewriter.create(
 insertSliceOp.getLoc(), newSrcType, insertSliceOp.getSource());
+
 rewriter.replaceOpWithNewOp(
 insertSliceOp, cast, insertSliceOp.getDest(),
 insertSliceOp.getMixedOffsets(), insertSliceOp.getMixedSizes(),
diff --git a/mlir/test/Dialect/Tensor/canonicalize.mlir 
b/mlir/test/Dialect/Tensor/canonicalize.mlir
index ea8c17640d7c1..88f27d3d36b04 100644
--- a/mlir/test/Dialect/Tensor/canonicalize.mlir
+++ b/mlir/test/Dialect/Tensor/canonicalize.mlir
@@ -1102,6 +1102,16 @@ func.func @no_fold_collapse_of_expand_empty_expr(%arg0: 
tensor<3x2x2xf32>)
 
 // -
 
+func.func @no_fold_extract_slice_negative_offset(%arg0: tensor<8xf32>) -> 
tensor {
+  %c-1 = arith.constant -1 : index
+  %e = tensor.extract_slice %arg0[1] [%c-1] [1] : tensor<8xf32> to 
tensor
+  return %e : tensor
+}
+// CHECK-LABEL: func @no_fold_extract_slice_negative_offset
+// CHECK: tensor.extract_slice
+
+// -
+
 func.func @reshape_splat_constant_int32() -> tensor<2x4x2xi32> {
   %c0 = arith.constant dense<42> : tensor<2x8xi32>
   %0 = tensor.expand_shape %c0 [[0], [1, 2]]

>From ecef5428c160cb72103e06a160c450440ce1f416 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 20 Nov 2023 16:27:53 +0100

[flang] [mlir] [compiler-rt] [clang-tools-extra] [clang] [libcxx] [llvm] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

@@ -0,0 +1,14 @@
+// RUN: mlir-opt %s -inline -split-input-file | FileCheck %s
+
+// CHECK-LABEL: func @inline_negative_stride
+func.func @inline_negative_stride(%arg0 : memref<10xf32>) -> memref<1xf32, 
strided<[?], offset: 1>> {
+  cf.br ^bb1(%arg0 : memref<10xf32>)
+^bb1(%m: memref<10xf32>):
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+  %1 = memref.subview %m[1] [1] [%c0] : memref<10xf32> to memref<1xf32, 
strided<[?], offset: 1>>
+  return %1 : memref<1xf32, strided<[?], offset: 1>>
+}
+// CHECK-NEXT: arith.constant 0 : index
+// CHECK-NEXT: %[[SUBVIEW:.*]] = memref.subview
+// CHECK-NEXT: return %[[SUBVIEW]] : memref<1xf32, strided<[?], offset: 1>>

rikhuijzer wrote:

I removed this file again. 👍 

https://github.com/llvm/llvm-project/pull/74200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[mlir] [llvm] [clang] [flang] [compiler-rt] [clang-tools-extra] [libcxx] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/74200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[mlir] [llvm] [clang] [flang] [compiler-rt] [clang-tools-extra] [libcxx] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/74200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [libcxx] [clang-tools-extra] [flang] [libc] [mlir] [clang] [llvm] [mlir][llvm] Fix verifier for const int and dense (PR #74340)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/74340

>From 47140903675a902a5803ef170bcd928783217a5f Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 4 Dec 2023 16:57:08 +0100
Subject: [PATCH 1/4] [mlir][llvm] Fix verifier for const int

---
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp  | 4 
 mlir/test/Target/LLVMIR/llvmir-invalid.mlir | 8 
 2 files changed, 12 insertions(+)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp 
b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 71860363b8ea5..b1c1c2ba04e4c 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2539,6 +2539,10 @@ LogicalResult LLVM::ConstantOp::verify() {
   if (!llvm::isa(getValue()))
 return emitOpError()
<< "only supports integer, float, string or elements attributes";
+  if (auto intAttr = dyn_cast(getValue())) {
+if (!llvm::isa(getType()))
+  return emitOpError() << "expected integer constant to have integer type";
+  }
   if (auto floatAttr = dyn_cast(getValue())) {
 const llvm::fltSemantics &sem = floatAttr.getValue().getSemantics();
 unsigned floatWidth = APFloat::getSizeInBits(sem);
diff --git a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir 
b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
index 0def5895fb330..fb1fb7cf13d7f 100644
--- a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
@@ -31,6 +31,14 @@ llvm.func @struct_wrong_attribute_element_type() -> 
!llvm.struct<(f64, f64)> {
 
 // -
 
+llvm.func @integer_with_float_type() -> f32 {
+  // expected-error @+1 {{expected integer constant to have integer type}}
+  %0 = llvm.mlir.constant(1 : index) : f32
+  llvm.return %0 : f32
+}
+
+// -
+
 llvm.func @incompatible_float_attribute_type() -> f32 {
   // expected-error @below{{expected float type of width 64}}
   %cst = llvm.mlir.constant(1.0 : f64) : f32

>From ee0a857ab3fb2e93e51d56d565d29e79f7cdcb67 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 4 Dec 2023 17:40:32 +0100
Subject: [PATCH 2/4] Throw error on vector attr with float type

---
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp  |  7 ++-
 mlir/test/Target/LLVMIR/llvmir-invalid.mlir | 10 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp 
b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index b1c1c2ba04e4c..705788d237d7f 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2541,7 +2541,7 @@ LogicalResult LLVM::ConstantOp::verify() {
<< "only supports integer, float, string or elements attributes";
   if (auto intAttr = dyn_cast(getValue())) {
 if (!llvm::isa(getType()))
-  return emitOpError() << "expected integer constant to have integer type";
+  return emitOpError() << "expected integer type";
   }
   if (auto floatAttr = dyn_cast(getValue())) {
 const llvm::fltSemantics &sem = floatAttr.getValue().getSemantics();
@@ -2557,6 +2557,11 @@ LogicalResult LLVM::ConstantOp::verify() {
   return emitOpError() << "expected integer type of width " << floatWidth;
 }
   }
+  if (auto splatAttr = dyn_cast(getValue())) {
+if (!getType().isa() && !getType().isa() 
&&
+!getType().isa() && 
!getType().isa())
+  return emitOpError() << "expected vector or array type";
+  }
   return success();
 }
 
diff --git a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir 
b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
index fb1fb7cf13d7f..117a6b8269089 100644
--- a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
@@ -7,6 +7,14 @@ func.func @foo() {
 
 // -
 
+llvm.func @vector_with_non_vector_type() -> f32 {
+  // expected-error @below{{expected vector or array type}}
+  %cst = llvm.mlir.constant(dense<100.0> : vector<1xf64>) : f32
+  llvm.return %cst : f32
+}
+
+// -
+
 llvm.func @no_non_complex_struct() -> !llvm.array<2 x array<2 x array<2 x 
struct<(i32) {
   // expected-error @below{{expected struct type to be a complex number}}
   %0 = llvm.mlir.constant(dense<[[[1, 2], [3, 4]], [[42, 43], [44, 45]]]> : 
tensor<2x2x2xi32>) : !llvm.array<2 x array<2 x array<2 x struct<(i32)
@@ -32,7 +40,7 @@ llvm.func @struct_wrong_attribute_element_type() -> 
!llvm.struct<(f64, f64)> {
 // -
 
 llvm.func @integer_with_float_type() -> f32 {
-  // expected-error @+1 {{expected integer constant to have integer type}}
+  // expected-error @+1 {{expected integer type}}
   %0 = llvm.mlir.constant(1 : index) : f32
   llvm.return %0 : f32
 }

>From f55e793aa1f6daa4cd56ebd63dd65d53159fcc20 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 4 Dec 2023 18:02:13 +0100
Subject: [PATCH 3/4] Apply `clang-format`

---
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp 
b/mlir/lib/Dialect/LLVMIR/IR

[llvm] [libc] [mlir] [clang-tools-extra] [clang] [libcxx] [compiler-rt] [flang] [mlir][llvm] Fix verifier for const int and dense (PR #74340)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/74340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[mlir] [flang] [llvm] [compiler-rt] [libcxx] [clang-tools-extra] [clang] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/74200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [compiler-rt] [llvm] [flang] [mlir] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/72059

>From 8f90ae9113229faf5ab4b859e5c70f23853f4db8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sat, 11 Nov 2023 20:43:31 +0100
Subject: [PATCH 01/12] [mlir][memref] Verify subview offsets and sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 12 +++-
 mlir/test/Dialect/MemRef/invalid.mlir| 16 
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 215a8f5e7d18be0..dc3e04be83fc1c9 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2842,8 +2842,18 @@ static LogicalResult 
produceSubViewErrorMsg(SliceVerificationResult result,
   llvm_unreachable("unexpected subview verification result");
 }
 
-/// Verifier for SubViewOp.
 LogicalResult SubViewOp::verify() {
+  for (int64_t offset : getStaticOffsets()) {
+if (offset < 0 && !ShapedType::isDynamic(offset))
+  return emitError("expected subview offsets to be non-negative, but got ")
+ << offset;
+  }
+  for (int64_t size : getStaticSizes()) {
+if (size < 1 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be positive, but got ")
+ << size;
+  }
+
   MemRefType baseType = getSourceType();
   MemRefType subViewType = getType();
 
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index cb5977e302a993f..17f5fbea3bda17a 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -611,6 +611,22 @@ func.func @invalid_view(%arg0 : index, %arg1 : index, 
%arg2 : index) {
 
 // -
 
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview offsets to be non-negative, but got 
-1}}
+  %0 = memref.subview %input[-1, 256] [2, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
+  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
+  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -
+
 func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
   %0 = memref.alloc() : memref<8x16x4xf32>
   // expected-error@+1 {{expected mixed offsets rank to match mixed sizes rank 
(2 vs 3) so the rank of the result type is well-formed}}

>From 32adf97c8d19e29e4673c772bd190c0eb0b413e9 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sun, 12 Nov 2023 09:10:00 +0100
Subject: [PATCH 02/12] Allow zero sizes

---
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 5 +++--
 mlir/test/Dialect/MemRef/invalid.mlir| 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp 
b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index dc3e04be83fc1c9..a24467a321a70c5 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2849,8 +2849,8 @@ LogicalResult SubViewOp::verify() {
  << offset;
   }
   for (int64_t size : getStaticSizes()) {
-if (size < 1 && !ShapedType::isDynamic(size))
-  return emitError("expected subview sizes to be positive, but got ")
+if (size < 0 && !ShapedType::isDynamic(size))
+  return emitError("expected subview sizes to be non-negative, but got ")
  << size;
   }
 
@@ -3103,6 +3103,7 @@ struct SubViewReturnTypeCanonicalizer {
   MemRefType operator()(SubViewOp op, ArrayRef mixedOffsets,
 ArrayRef mixedSizes,
 ArrayRef mixedStrides) {
+
 // Infer a memref type without taking into account any rank reductions.
 MemRefType nonReducedType = cast(SubViewOp::inferResultType(
 op.getSourceType(), mixedOffsets, mixedSizes, mixedStrides));
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir 
b/mlir/test/Dialect/MemRef/invalid.mlir
index 17f5fbea3bda17a..38c0bcc3f2491c2 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -620,8 +620,8 @@ func.func @invalid_subview(%input: memref<4x1024xf32>) -> 
memref<2x256xf32, stri
 // -
 
 func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, 
strided<[1024, 1], offset: 2304>> {
-  // expected-error@+1 {{expected subview sizes to be positive, but got 0}}
-  %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to 
memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+  // expected-error@+1 {{expected subview sizes to be non-negative, b

[mlir] [llvm] [clang] [flang] [compiler-rt] [clang-tools-extra] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[mlir] [llvm] [clang] [flang] [compiler-rt] [clang-tools-extra] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

rikhuijzer wrote:

> Both options are correct, but they should be consistent. Not folding negative 
> dimension seems like a straightforward thing I think?

After finding the right minimal reproducer, it was yes. Thanks.

I've updated the first comment of this PR now that folding on negative 
dimensions will be avoided.

https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [flang] [clang] [mlir] [compiler-rt] [clang-tools-extra] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [clang-tools-extra] [clang] [llvm] [mlir] [flang] [mlir] Verify non-negative `offset` and `size` (PR #72059)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/72059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [lld] [lldb] [compiler-rt] [mlir] [clang] [flang] [libcxx] [mlir][async] Avoid crash when not using `func.func` (PR #72801)

[Rik Huijzer via cfe-commits]

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

[llvm] [lld] [lldb] [compiler-rt] [mlir] [clang] [flang] [libcxx] [mlir][async] Avoid crash when not using `func.func` (PR #72801)

[Rik Huijzer via cfe-commits]

@@ -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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-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)

[Rik Huijzer via cfe-commits]

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

[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)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/72801
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] [mlir] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer created 
https://github.com/llvm/llvm-project/pull/74200

This PR fixes https://github.com/llvm/llvm-project/issues/73383 and is another 
shot at the refactoring proposed in 
https://github.com/llvm/llvm-project/pull/72885.

>From 22928e7e5da508d8d9dc8d4b7e54f84cccadef06 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 20 Nov 2023 09:02:41 +0100
Subject: [PATCH 1/5] [mlir][tensor] Fix canon via `hasNegativeDimension`

---
 mlir/include/mlir/Dialect/Tensor/IR/Tensor.h |  6 ++
 mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 15 +++
 mlir/test/Dialect/Tensor/canonicalize.mlir   | 10 ++
 3 files changed, 31 insertions(+)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h 
b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
index 06642adda42b3..0d027057b3a95 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
+++ b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
@@ -150,6 +150,12 @@ LogicalResult getOrCreateDestinations(OpBuilder &b, 
Location loc, Operation *op,
 /// Tests if types are the same when ignoring encoding on ranked tensors.
 bool isSameTypeWithoutEncoding(Type tp1, Type tp2);
 
+/// Helper function to check whether the dimensions are non-negative. This
+/// check also occurs in the verifier, but we need it at later stages too
+/// because the verifier ignores dynamic dimensions, but later stages might
+/// have constant folded those to (negative) constants.
+bool hasNegativeDimension(SmallVector shape);
+
 /// Function to control the folding of constant and extract slice.
 using ControlConstantExtractSliceFusionFn = 
std::function;
 
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp 
b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index e469815496e18..3297ef673ca2e 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -125,6 +125,12 @@ bool tensor::isSameTypeWithoutEncoding(Type tp1, Type tp2) 
{
   return tp1 == tp2; // default implementation
 }
 
+bool tensor::hasNegativeDimension(SmallVector shape) {
+  return llvm::any_of(shape, [](int64_t dim) {
+return !ShapedType::isDynamic(dim) && dim < 0;
+  });
+}
+
 /// Compute the dropped dimensions of a rank-reducing tensor.extract_slice op 
or
 /// rank-extending tensor.insert_slice op.
 static llvm::SmallBitVector getDroppedDims(ArrayRef reducedShape,
@@ -1801,6 +1807,10 @@ RankedTensorType 
ExtractSliceOp::inferCanonicalRankReducedResultType(
   dispatchIndexOpFoldResults(offsets, dynamicOffsets, staticOffsets);
   dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
   dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
+  if (hasNegativeDimension(staticOffsets))
+return {};
+  if (hasNegativeDimension(staticSizes))
+return {};
   return ExtractSliceOp::inferCanonicalRankReducedResultType(
   desiredResultRank, sourceRankedTensorType, staticOffsets, staticSizes,
   staticStrides);
@@ -2370,6 +2380,8 @@ class InsertSliceOpConstantArgumentFolder final
 auto sourceType = ExtractSliceOp::inferCanonicalRankReducedResultType(
 insertSliceOp.getSourceType().getRank(), insertSliceOp.getDestType(),
 mixedOffsets, mixedSizes, mixedStrides);
+if (!sourceType)
+  return failure();
 Value toInsert = insertSliceOp.getSource();
 if (sourceType != insertSliceOp.getSourceType()) {
   OpBuilder::InsertionGuard g(rewriter);
@@ -2500,6 +2512,8 @@ struct InsertSliceOpSourceCastInserter final
   getConstantIntValue(insertSliceOp.getMixedSizes()[i]))
 newSrcShape[i] = *constInt;
 }
+// if (hasNegativeDimension(newSrcShape))
+//  return failure();
 
 RankedTensorType newSrcType =
 RankedTensorType::get(newSrcShape, srcType.getElementType());
@@ -2521,6 +2535,7 @@ struct InsertSliceOpSourceCastInserter final
   rewriter.setInsertionPoint(insertSliceOp->getParentOp());
 Value cast = rewriter.create(
 insertSliceOp.getLoc(), newSrcType, insertSliceOp.getSource());
+
 rewriter.replaceOpWithNewOp(
 insertSliceOp, cast, insertSliceOp.getDest(),
 insertSliceOp.getMixedOffsets(), insertSliceOp.getMixedSizes(),
diff --git a/mlir/test/Dialect/Tensor/canonicalize.mlir 
b/mlir/test/Dialect/Tensor/canonicalize.mlir
index ea8c17640d7c1..88f27d3d36b04 100644
--- a/mlir/test/Dialect/Tensor/canonicalize.mlir
+++ b/mlir/test/Dialect/Tensor/canonicalize.mlir
@@ -1102,6 +1102,16 @@ func.func @no_fold_collapse_of_expand_empty_expr(%arg0: 
tensor<3x2x2xf32>)
 
 // -
 
+func.func @no_fold_extract_slice_negative_offset(%arg0: tensor<8xf32>) -> 
tensor {
+  %c-1 = arith.constant -1 : index
+  %e = tensor.extract_slice %arg0[1] [%c-1] [1] : tensor<8xf32> to 
tensor
+  return %e : tensor
+}
+// CHECK-LABEL: func @no_fold_extract_slice_negative_offset
+// CHECK: tensor.extract_slice
+
+// -
+
 func.func @reshape_splat_constant_int32() -> tensor<2x4x2xi32> {
   %c0 = arith.constant dense<42> : tensor<2x8xi32>
   %0 = 

[clang-tools-extra] [mlir] [llvm] [clang] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/74200

>From 22928e7e5da508d8d9dc8d4b7e54f84cccadef06 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 20 Nov 2023 09:02:41 +0100
Subject: [PATCH 1/6] [mlir][tensor] Fix canon via `hasNegativeDimension`

---
 mlir/include/mlir/Dialect/Tensor/IR/Tensor.h |  6 ++
 mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 15 +++
 mlir/test/Dialect/Tensor/canonicalize.mlir   | 10 ++
 3 files changed, 31 insertions(+)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h 
b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
index 06642adda42b3..0d027057b3a95 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
+++ b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
@@ -150,6 +150,12 @@ LogicalResult getOrCreateDestinations(OpBuilder &b, 
Location loc, Operation *op,
 /// Tests if types are the same when ignoring encoding on ranked tensors.
 bool isSameTypeWithoutEncoding(Type tp1, Type tp2);
 
+/// Helper function to check whether the dimensions are non-negative. This
+/// check also occurs in the verifier, but we need it at later stages too
+/// because the verifier ignores dynamic dimensions, but later stages might
+/// have constant folded those to (negative) constants.
+bool hasNegativeDimension(SmallVector shape);
+
 /// Function to control the folding of constant and extract slice.
 using ControlConstantExtractSliceFusionFn = 
std::function;
 
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp 
b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index e469815496e18..3297ef673ca2e 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -125,6 +125,12 @@ bool tensor::isSameTypeWithoutEncoding(Type tp1, Type tp2) 
{
   return tp1 == tp2; // default implementation
 }
 
+bool tensor::hasNegativeDimension(SmallVector shape) {
+  return llvm::any_of(shape, [](int64_t dim) {
+return !ShapedType::isDynamic(dim) && dim < 0;
+  });
+}
+
 /// Compute the dropped dimensions of a rank-reducing tensor.extract_slice op 
or
 /// rank-extending tensor.insert_slice op.
 static llvm::SmallBitVector getDroppedDims(ArrayRef reducedShape,
@@ -1801,6 +1807,10 @@ RankedTensorType 
ExtractSliceOp::inferCanonicalRankReducedResultType(
   dispatchIndexOpFoldResults(offsets, dynamicOffsets, staticOffsets);
   dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
   dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
+  if (hasNegativeDimension(staticOffsets))
+return {};
+  if (hasNegativeDimension(staticSizes))
+return {};
   return ExtractSliceOp::inferCanonicalRankReducedResultType(
   desiredResultRank, sourceRankedTensorType, staticOffsets, staticSizes,
   staticStrides);
@@ -2370,6 +2380,8 @@ class InsertSliceOpConstantArgumentFolder final
 auto sourceType = ExtractSliceOp::inferCanonicalRankReducedResultType(
 insertSliceOp.getSourceType().getRank(), insertSliceOp.getDestType(),
 mixedOffsets, mixedSizes, mixedStrides);
+if (!sourceType)
+  return failure();
 Value toInsert = insertSliceOp.getSource();
 if (sourceType != insertSliceOp.getSourceType()) {
   OpBuilder::InsertionGuard g(rewriter);
@@ -2500,6 +2512,8 @@ struct InsertSliceOpSourceCastInserter final
   getConstantIntValue(insertSliceOp.getMixedSizes()[i]))
 newSrcShape[i] = *constInt;
 }
+// if (hasNegativeDimension(newSrcShape))
+//  return failure();
 
 RankedTensorType newSrcType =
 RankedTensorType::get(newSrcShape, srcType.getElementType());
@@ -2521,6 +2535,7 @@ struct InsertSliceOpSourceCastInserter final
   rewriter.setInsertionPoint(insertSliceOp->getParentOp());
 Value cast = rewriter.create(
 insertSliceOp.getLoc(), newSrcType, insertSliceOp.getSource());
+
 rewriter.replaceOpWithNewOp(
 insertSliceOp, cast, insertSliceOp.getDest(),
 insertSliceOp.getMixedOffsets(), insertSliceOp.getMixedSizes(),
diff --git a/mlir/test/Dialect/Tensor/canonicalize.mlir 
b/mlir/test/Dialect/Tensor/canonicalize.mlir
index ea8c17640d7c1..88f27d3d36b04 100644
--- a/mlir/test/Dialect/Tensor/canonicalize.mlir
+++ b/mlir/test/Dialect/Tensor/canonicalize.mlir
@@ -1102,6 +1102,16 @@ func.func @no_fold_collapse_of_expand_empty_expr(%arg0: 
tensor<3x2x2xf32>)
 
 // -
 
+func.func @no_fold_extract_slice_negative_offset(%arg0: tensor<8xf32>) -> 
tensor {
+  %c-1 = arith.constant -1 : index
+  %e = tensor.extract_slice %arg0[1] [%c-1] [1] : tensor<8xf32> to 
tensor
+  return %e : tensor
+}
+// CHECK-LABEL: func @no_fold_extract_slice_negative_offset
+// CHECK: tensor.extract_slice
+
+// -
+
 func.func @reshape_splat_constant_int32() -> tensor<2x4x2xi32> {
   %c0 = arith.constant dense<42> : tensor<2x8xi32>
   %0 = tensor.expand_shape %c0 [[0], [1, 2]]

>From ecef5428c160cb72103e06a160c450440ce1f416 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 20 Nov 2023 16:27:53 +0100

[libcxx] [clang] [llvm] [mlir] [compiler-rt] [flang] [clang-tools-extra] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/74200

>From 22928e7e5da508d8d9dc8d4b7e54f84cccadef06 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 20 Nov 2023 09:02:41 +0100
Subject: [PATCH 1/7] [mlir][tensor] Fix canon via `hasNegativeDimension`

---
 mlir/include/mlir/Dialect/Tensor/IR/Tensor.h |  6 ++
 mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 15 +++
 mlir/test/Dialect/Tensor/canonicalize.mlir   | 10 ++
 3 files changed, 31 insertions(+)

diff --git a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h 
b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
index 06642adda42b3..0d027057b3a95 100644
--- a/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
+++ b/mlir/include/mlir/Dialect/Tensor/IR/Tensor.h
@@ -150,6 +150,12 @@ LogicalResult getOrCreateDestinations(OpBuilder &b, 
Location loc, Operation *op,
 /// Tests if types are the same when ignoring encoding on ranked tensors.
 bool isSameTypeWithoutEncoding(Type tp1, Type tp2);
 
+/// Helper function to check whether the dimensions are non-negative. This
+/// check also occurs in the verifier, but we need it at later stages too
+/// because the verifier ignores dynamic dimensions, but later stages might
+/// have constant folded those to (negative) constants.
+bool hasNegativeDimension(SmallVector shape);
+
 /// Function to control the folding of constant and extract slice.
 using ControlConstantExtractSliceFusionFn = 
std::function;
 
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp 
b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index e469815496e18..3297ef673ca2e 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -125,6 +125,12 @@ bool tensor::isSameTypeWithoutEncoding(Type tp1, Type tp2) 
{
   return tp1 == tp2; // default implementation
 }
 
+bool tensor::hasNegativeDimension(SmallVector shape) {
+  return llvm::any_of(shape, [](int64_t dim) {
+return !ShapedType::isDynamic(dim) && dim < 0;
+  });
+}
+
 /// Compute the dropped dimensions of a rank-reducing tensor.extract_slice op 
or
 /// rank-extending tensor.insert_slice op.
 static llvm::SmallBitVector getDroppedDims(ArrayRef reducedShape,
@@ -1801,6 +1807,10 @@ RankedTensorType 
ExtractSliceOp::inferCanonicalRankReducedResultType(
   dispatchIndexOpFoldResults(offsets, dynamicOffsets, staticOffsets);
   dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
   dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
+  if (hasNegativeDimension(staticOffsets))
+return {};
+  if (hasNegativeDimension(staticSizes))
+return {};
   return ExtractSliceOp::inferCanonicalRankReducedResultType(
   desiredResultRank, sourceRankedTensorType, staticOffsets, staticSizes,
   staticStrides);
@@ -2370,6 +2380,8 @@ class InsertSliceOpConstantArgumentFolder final
 auto sourceType = ExtractSliceOp::inferCanonicalRankReducedResultType(
 insertSliceOp.getSourceType().getRank(), insertSliceOp.getDestType(),
 mixedOffsets, mixedSizes, mixedStrides);
+if (!sourceType)
+  return failure();
 Value toInsert = insertSliceOp.getSource();
 if (sourceType != insertSliceOp.getSourceType()) {
   OpBuilder::InsertionGuard g(rewriter);
@@ -2500,6 +2512,8 @@ struct InsertSliceOpSourceCastInserter final
   getConstantIntValue(insertSliceOp.getMixedSizes()[i]))
 newSrcShape[i] = *constInt;
 }
+// if (hasNegativeDimension(newSrcShape))
+//  return failure();
 
 RankedTensorType newSrcType =
 RankedTensorType::get(newSrcShape, srcType.getElementType());
@@ -2521,6 +2535,7 @@ struct InsertSliceOpSourceCastInserter final
   rewriter.setInsertionPoint(insertSliceOp->getParentOp());
 Value cast = rewriter.create(
 insertSliceOp.getLoc(), newSrcType, insertSliceOp.getSource());
+
 rewriter.replaceOpWithNewOp(
 insertSliceOp, cast, insertSliceOp.getDest(),
 insertSliceOp.getMixedOffsets(), insertSliceOp.getMixedSizes(),
diff --git a/mlir/test/Dialect/Tensor/canonicalize.mlir 
b/mlir/test/Dialect/Tensor/canonicalize.mlir
index ea8c17640d7c1..88f27d3d36b04 100644
--- a/mlir/test/Dialect/Tensor/canonicalize.mlir
+++ b/mlir/test/Dialect/Tensor/canonicalize.mlir
@@ -1102,6 +1102,16 @@ func.func @no_fold_collapse_of_expand_empty_expr(%arg0: 
tensor<3x2x2xf32>)
 
 // -
 
+func.func @no_fold_extract_slice_negative_offset(%arg0: tensor<8xf32>) -> 
tensor {
+  %c-1 = arith.constant -1 : index
+  %e = tensor.extract_slice %arg0[1] [%c-1] [1] : tensor<8xf32> to 
tensor
+  return %e : tensor
+}
+// CHECK-LABEL: func @no_fold_extract_slice_negative_offset
+// CHECK: tensor.extract_slice
+
+// -
+
 func.func @reshape_splat_constant_int32() -> tensor<2x4x2xi32> {
   %c0 = arith.constant dense<42> : tensor<2x8xi32>
   %0 = tensor.expand_shape %c0 [[0], [1, 2]]

>From ecef5428c160cb72103e06a160c450440ce1f416 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Mon, 20 Nov 2023 16:27:53 +0100

[libcxx] [clang] [llvm] [mlir] [compiler-rt] [flang] [clang-tools-extra] [mlir] Fix a zero stride canonicalizer crash (PR #74200)

[Rik Huijzer via cfe-commits]

rikhuijzer wrote:

> Is it possible to add the test case (or minimal similar example) with 
> `--inline` option so that we can confirm the original issue is resolved.
> 
> #73383

Thanks for the review and fixing the typos that I've made! I've added a test in 
a new `MemRef/inlining.mlir` file, which is in line with the other files:
```
$ rg -l '\-inline' mlir/test
mlir/test/Dialect/MemRef/inlining.mlir
mlir/test/Dialect/Vector/inlining.mlir
mlir/test/Dialect/UB/inlining.mlir
mlir/test/Dialect/Async/async-parallel-for-compute-fn.mlir
mlir/test/Dialect/Async/async-parallel-for-canonicalize.mlir
mlir/test/Dialect/Affine/inlining.mlir
mlir/test/Dialect/LLVMIR/inline-byval-huge.mlir
mlir/test/Dialect/Bufferization/inlining.mlir
mlir/test/Dialect/LLVMIR/inlining-alias-scopes.mlir
mlir/test/Dialect/LLVMIR/inlining.mlir
mlir/test/Dialect/Linalg/inline-scalar-operands.mlir
mlir/test/Dialect/Tosa/inlining.mlir
mlir/test/Dialect/Linalg/inlining.mlir
mlir/test/lib/Transforms/TestInlining.cpp
mlir/test/Transforms/inlining.mlir
mlir/test/Transforms/inlining-repeated-use.mlir
mlir/test/Transforms/inlining-recursive.mlir
mlir/test/Transforms/inlining-dce.mlir
mlir/test/Transforms/test-inlining.mlir
```
(Also test additions can always be reverted of course so it shouldn't be too 
bad.) Also I double checked that 
https://github.com/llvm/llvm-project/issues/73383 doesn't crash on this PR.

https://github.com/llvm/llvm-project/pull/74200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [mlir] [compiler-rt] [clang-tools-extra] [flang] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)

[Rik Huijzer via cfe-commits]

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] [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,
+

[mlir] [flang] [clang] [compiler-rt] [llvm] [clang-tools-extra] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/76292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[mlir] [flang] [clang] [compiler-rt] [llvm] [clang-tools-extra] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/76292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[mlir] [flang] [clang] [compiler-rt] [llvm] [clang-tools-extra] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer edited 
https://github.com/llvm/llvm-project/pull/76292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [flang] [mlir] [llvm] [clang-tools-extra] [clang] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)

[Rik Huijzer via cfe-commits]

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,
+

[compiler-rt] [flang] [mlir] [llvm] [clang-tools-extra] [clang] [mlir][vector] Fix invalid `LoadOp` indices being created (PR #76292)

[Rik Huijzer via cfe-commits]

@@ -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:

That's caused by git diff being confusing. The `maskBufferType` was defined 
before the `if-else`, but only used inside the `else`, so I moved it inside the 
`else`.

https://github.com/llvm/llvm-project/pull/76292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-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)

[Rik Huijzer via cfe-commits]

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,
+

[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)

[Rik Huijzer via cfe-commits]

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,
+

[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)

[Rik Huijzer via cfe-commits]

@@ -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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-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)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/76292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-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)

[Rik Huijzer via cfe-commits]

rikhuijzer wrote:

@joker-eph, thanks again for the review!

https://github.com/llvm/llvm-project/pull/76292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [mlir] Verify TestBuiltinAttributeInterfaces eltype (PR #69878)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer created 
https://github.com/llvm/llvm-project/pull/69878

Fixes https://github.com/llvm/llvm-project/issues/61871.

This PR fixes two small things. First and foremost, it throws a clear error in 
the `-test-elements-attr-interface` when those tests are called on elements 
which are not an integer. I've looked through the introduction of the attribute 
interface (https://reviews.llvm.org/D109190) and later commits and see no 
evidence that the interface (`attr.tryGetValues()`) is expected to handle 
mismatching types. 

For example, the case which is given in the issue is:
```mlir
arith.constant sparse<[[0, 0, 5]],  -2.0> : vector<1x1x10xf16>
```
So, a sparse vector containing `f16` elements. This will crash at various 
locations when called in the test because the test introduces integer types 
(`int64_t`, `uint64_t`, `APInt`, `IntegerAttr`), but as I said in the previous 
paragraph: I see no reason to believe that the interface is wrong here. The 
interface assumes that clients don't do things like
`attr.tryGetValues()` on a floating point `attr`.

Also I've added a test for the implementation of this interface by the `sparse` 
dialect. There were no problems there. Still, probably good to increase code 
coverage on that one (also, to avoid new issue reports like 
https://github.com/llvm/llvm-project/issues/61871).

>From 1d27516523cde11767705396b0ea5a9e1f264f50 Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Sun, 22 Oct 2023 14:36:59 +0200
Subject: [PATCH] [mlir] Verify TestBuiltinAttributeInterfaces eltype

---
 mlir/test/IR/elements-attr-interface.mlir   | 13 +
 mlir/test/lib/IR/TestBuiltinAttributeInterfaces.cpp |  5 +
 2 files changed, 18 insertions(+)

diff --git a/mlir/test/IR/elements-attr-interface.mlir 
b/mlir/test/IR/elements-attr-interface.mlir
index e5f17d043f1aace..5234c81bd841e39 100644
--- a/mlir/test/IR/elements-attr-interface.mlir
+++ b/mlir/test/IR/elements-attr-interface.mlir
@@ -20,6 +20,13 @@ arith.constant #test.i64_elements<[10, 11, 12, 13, 14]> : 
tensor<5xi64>
 // expected-error@below {{Test iterating `IntegerAttr`: 10 : i64, 11 : i64, 12 
: i64, 13 : i64, 14 : i64}}
 arith.constant dense<[10, 11, 12, 13, 14]> : tensor<5xi64>
 
+// This test is expected to only be called on integer elements.
+// expected-error@below {{Test iterating `int64_t`: expected element type to 
be an integer type}}
+// expected-error@below {{Test iterating `uint64_t`: expected element type to 
be an integer type}}
+// expected-error@below {{Test iterating `APInt`: expected element type to be 
an integer type}}
+// expected-error@below {{Test iterating `IntegerAttr`: expected element type 
to be an integer type}}
+arith.constant dense<[1.1, 1.2, 1.3]> : tensor<3xf32>
+
 // Check that we don't crash on empty element attributes.
 // expected-error@below {{Test iterating `int64_t`: }}
 // expected-error@below {{Test iterating `uint64_t`: }}
@@ -41,3 +48,9 @@ arith.constant #test.e1di64_elements : tensor<3xi64>
 }
   }
 #-}
+
+// expected-error@below {{Test iterating `int64_t`: 0, 0, 1}}
+// expected-error@below {{Test iterating `uint64_t`: 0, 0, 1}}
+// expected-error@below {{Test iterating `APInt`: 0, 0, 1}}
+// expected-error@below {{Test iterating `IntegerAttr`: 0 : i64, 0 : i64, 1 : 
i64}}
+arith.constant sparse<[[0, 0, 2]], 1> : vector <1x1x3xi64>
diff --git a/mlir/test/lib/IR/TestBuiltinAttributeInterfaces.cpp 
b/mlir/test/lib/IR/TestBuiltinAttributeInterfaces.cpp
index 498de3d87bd4bc3..71ed30bfbe34cd0 100644
--- a/mlir/test/lib/IR/TestBuiltinAttributeInterfaces.cpp
+++ b/mlir/test/lib/IR/TestBuiltinAttributeInterfaces.cpp
@@ -51,6 +51,11 @@ struct TestElementsAttrInterface
 InFlightDiagnostic diag = op->emitError()
   << "Test iterating `" << type << "`: ";
 
+if (!attr.getElementType().isa()) {
+  diag << "expected element type to be an integer type";
+  return;
+}
+
 auto values = attr.tryGetValues();
 if (!values) {
   diag << "unable to iterate type";

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [mlir] Verify TestBuiltinAttributeInterfaces eltype (PR #69878)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/69878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir] Verify TestBuiltinAttributeInterfaces eltype (PR #69878)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/69878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir][llvm] Fix elem type passing into `getelementptr` (PR #68136)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer updated 
https://github.com/llvm/llvm-project/pull/68136

>From 57ec61b03b4c54164f40be996b1d57420f2d4a2c Mon Sep 17 00:00:00 2001
From: Rik Huijzer 
Date: Tue, 3 Oct 2023 19:31:03 +0200
Subject: [PATCH] [mlir][llvm] Fix elem type passing into `getelementptr`

---
 mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 9 +++--
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp  | 9 ++---
 mlir/test/Target/LLVMIR/opaque-ptr.mlir | 9 +
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td 
b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 25209ce4497455e..9d46c5b3bdc5e1b 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -303,8 +303,13 @@ def LLVM_GEPOp : LLVM_Op<"getelementptr", [Pure,
 indices.push_back(
 builder.getInt32(valueOrAttr.get().getInt()));
 }
-Type baseElementType = op.getSourceElementType();
-llvm::Type *elementType = moduleTranslation.convertType(baseElementType);
+
+Type elemTypeFromAttr = op.getSourceElementType();
+auto ptrType = ::llvm::cast(op.getType());
+Type elemTypeFromPtrType = ptrType.getElementType();
+
+llvm::Type *elementType = moduleTranslation.convertType(
+elemTypeFromAttr ? elemTypeFromAttr : elemTypeFromPtrType);
 $res = builder.CreateGEP(elementType, $base, indices, "", $inbounds);
   }];
   let assemblyFormat = [{
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp 
b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 95c04098d05fc2f..62cb595069e6652 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -287,14 +287,17 @@ ParseResult AllocaOp::parse(OpAsmParser &parser, 
OperationState &result) {
 }
 
 /// Checks that the elemental type is present in either the pointer type or
-/// the attribute, but not both.
+/// the attribute, but not in none or both.
 static LogicalResult verifyOpaquePtr(Operation *op, LLVMPointerType ptrType,
  std::optional ptrElementType) {
-  if (ptrType.isOpaque() && !ptrElementType.has_value()) {
+  bool typePresentInPointerType = !ptrType.isOpaque();
+  bool typePresentInAttribute = ptrElementType.has_value();
+
+  if (!typePresentInPointerType && !typePresentInAttribute) {
 return op->emitOpError() << "expected '" << kElemTypeAttrName
  << "' attribute if opaque pointer type is used";
   }
-  if (!ptrType.isOpaque() && ptrElementType.has_value()) {
+  if (typePresentInPointerType && typePresentInAttribute) {
 return op->emitOpError()
<< "unexpected '" << kElemTypeAttrName
<< "' attribute when non-opaque pointer type is used";
diff --git a/mlir/test/Target/LLVMIR/opaque-ptr.mlir 
b/mlir/test/Target/LLVMIR/opaque-ptr.mlir
index c21f9b0542debc6..3bde192b4cc4d02 100644
--- a/mlir/test/Target/LLVMIR/opaque-ptr.mlir
+++ b/mlir/test/Target/LLVMIR/opaque-ptr.mlir
@@ -42,6 +42,15 @@ llvm.func @opaque_ptr_gep_struct(%arg0: !llvm.ptr, %arg1: 
i32) -> !llvm.ptr {
   llvm.return %0 : !llvm.ptr
 }
 
+// CHECK-LABEL: @opaque_ptr_elem_type
+llvm.func @opaque_ptr_elem_type(%0: !llvm.ptr) -> !llvm.ptr {
+  // CHECK: getelementptr ptr, ptr
+  %1 = llvm.getelementptr %0[0] { elem_type = !llvm.ptr } : (!llvm.ptr) -> 
!llvm.ptr
+  // CHECK: getelementptr ptr, ptr
+  %2 = llvm.getelementptr %0[0] : (!llvm.ptr) -> !llvm.ptr
+  llvm.return %1 : !llvm.ptr
+}
+
 // CHECK-LABEL: @opaque_ptr_matrix_load_store
 llvm.func @opaque_ptr_matrix_load_store(%ptr: !llvm.ptr, %stride: i64) -> 
vector<48 x f32> {
   // CHECK: call <48 x float> @llvm.matrix.column.major.load.v48f32.i64

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir][llvm] Fix elem type passing into `getelementptr` (PR #68136)

[Rik Huijzer via cfe-commits]

rikhuijzer wrote:

> What are the use cases for allowing either of these two syntaxes?
> The LLVM Dialect tries to closely mirror LLVM proper as much as possible and 
> this would deviate from LLVMs behaviour. While the transition is currently 
> stalled, in the future typed pointers will be removed from the dialect 
> entirely, further discouraging the use of typed-pointers such as shown here.

>From reading , I assume that the use-case is 
>that "MLIR can afford to support both opaque and non-opaque pointers at the 
>same time and simplify the transition". But Alex (@ftynse) is probably the 
>best person to answer this.

Regardless of the big picture, the question might be unrelated to this PR as 
this PR only fixes a bug in the implementation. Currently, there are LLVM 
operations in MLIR that do accept both forms. For example, `llvm.alloca` where

```mlir
llvm.func @main(%sz : i64) {
  "llvm.alloca"(%sz) : (i64) -> !llvm.ptr
  llvm.return
}
```

and 

```mlir
llvm.func @main(%sz : i64) {
  "llvm.alloca"(%sz) { elem_type = i32 } : (i64) -> !llvm.ptr
  llvm.return
}
```
Both are translated to
```llvm
; ModuleID = 'LLVMDialectModule'
source_filename = "LLVMDialectModule"

declare ptr @malloc(i64)

declare void @free(ptr)

define void @main(i64 %0) {
  %2 = alloca i32, i64 %0, align 4
  ret void
}

!llvm.module.flags = !{!0}

!0 = !{i32 2, !"Debug Info Version", i32 3}
```
via
```sh
mlir-translate temp.mlir -mlir-to-llvmir
```
Conversely, `llvm.getelementptr` currently only accepts the attribute (`{ 
elem_type = !llvm.ptr }`). This PR makes things consistent again.

https://github.com/llvm/llvm-project/pull/68136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir][llvm] Fix elem type passing into `getelementptr` (PR #68136)

[Rik Huijzer via cfe-commits]

rikhuijzer wrote:

> [...] I am a bit afraid of the burden of having to maintain what is 
> essentially a third GEP representation.

Thanks for the explanation @zero9178. I am affraid I do not fully understand 
your reasoning (most likely due to a lack of knowledge on my side). However, it 
sounds very reasonable to me!

So before I spend more time on the code, shall I rewrite this PR to throw a 
clear error from `GEPOp::verify` for the incorrect representation? Then we can 
close https://github.com/llvm/llvm-project/issues/63832.

https://github.com/llvm/llvm-project/pull/68136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir][llvm] Fix elem type passing into `getelementptr` (PR #68136)

[Rik Huijzer via cfe-commits]

https://github.com/rikhuijzer closed 
https://github.com/llvm/llvm-project/pull/68136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir][llvm] Fix elem type passing into `getelementptr` (PR #68136)

[Rik Huijzer via cfe-commits]

rikhuijzer wrote:

Thanks both for the review and clarifications. I'm learning a lot.

https://github.com/llvm/llvm-project/pull/68136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)

[Rik Huijzer via cfe-commits]

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

[clang] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)

[Rik Huijzer via cfe-commits]

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

[clang] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)

[Rik Huijzer via cfe-commits]

@@ -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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits