Skip to content

[mlir][EmitC] Fail on memrefs with 0 dims in type conversion #111965

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

simon-camp
Copy link
Contributor

This let's the type conversion fail instead of generating invalid array types.

@llvmbot
Copy link
Member

llvmbot commented Oct 11, 2024

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Simon Camphausen (simon-camp)

Changes

This let's the type conversion fail instead of generating invalid array types.


Full diff: https://github.com/llvm/llvm-project/pull/111965.diff

2 Files Affected:

  • (modified) mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp (+3-1)
  • (modified) mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir (+16)
diff --git a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
index 2b7ac4b529cf0d..39532d34f616eb 100644
--- a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
+++ b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
@@ -167,7 +167,9 @@ void mlir::populateMemRefToEmitCTypeConversion(TypeConverter &typeConverter) {
   typeConverter.addConversion(
       [&](MemRefType memRefType) -> std::optional<Type> {
         if (!memRefType.hasStaticShape() ||
-            !memRefType.getLayout().isIdentity() || memRefType.getRank() == 0) {
+            !memRefType.getLayout().isIdentity() || memRefType.getRank() == 0 ||
+            llvm::any_of(memRefType.getShape(),
+                         [](int64_t dim) { return dim == 0; })) {
           return {};
         }
         Type convertedElementType =
diff --git a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
index dee9cc97a14493..fda01974d3fc85 100644
--- a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
+++ b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
@@ -41,6 +41,22 @@ func.func @zero_rank() {
 
 // -----
 
+func.func @zero_dim_rank_1() {
+  // expected-error@+1 {{failed to legalize operation 'memref.alloca'}}
+  %0 = memref.alloca() : memref<0xf32>
+  return
+}
+
+// -----
+
+func.func @zero_dim_rank_3() {
+  // expected-error@+1 {{failed to legalize operation 'memref.alloca'}}
+  %0 = memref.alloca() : memref<2x0x4xf32>
+  return
+}
+
+// -----
+
 // expected-error@+1 {{failed to legalize operation 'memref.global'}}
 memref.global "nested" constant @nested_global : memref<3x7xf32>
 

Copy link
Contributor

@mgehre-amd mgehre-amd left a comment

Choose a reason for hiding this comment

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

Thanks!

@simon-camp simon-camp merged commit 7771429 into llvm:main Oct 11, 2024
11 checks passed
@simon-camp simon-camp deleted the emitc.memref.zero.dim branch October 11, 2024 09:45
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…1965)

This let's the type conversion fail instead of generating invalid array
types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants