[Mlir-commits] [mlir] [mlirbc] Fix incorrect skipping for single byte blob. (PR #182445)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Jacques Pienaar (jpienaar)

<details>
<summary>Changes</summary>

This resulted in not correctly unpacking. Added additional regression tests.

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


3 Files Affected:

- (modified) mlir/lib/IR/BuiltinDialectBytecode.cpp (+2-3) 
- (added) mlir/test/Bytecode/i1_roundtrip.mlir (+45) 
- (removed) mlir/test/Bytecode/i1_splat_roundtrip.mlir (-17) 


``````````diff
diff --git a/mlir/lib/IR/BuiltinDialectBytecode.cpp b/mlir/lib/IR/BuiltinDialectBytecode.cpp
index a87224a48c32a..8cf25391ba11a 100644
--- a/mlir/lib/IR/BuiltinDialectBytecode.cpp
+++ b/mlir/lib/IR/BuiltinDialectBytecode.cpp
@@ -168,12 +168,11 @@ readDenseIntOrFPElementsAttr(DialectBytecodeReader &reader, ShapedType type,
   // cheap.
   size_t numElements = type.getNumElements();
   size_t packedSize = llvm::divideCeil(numElements, 8);
-  if (blob.size() == packedSize && blob.size() != numElements &&
-      blob.size() != 1) {
+  if (blob.size() == packedSize && blob.size() != numElements) {
     // Unpack the blob.
     rawData.resize(numElements);
     for (size_t i = 0; i < numElements; ++i)
-      rawData[i] = (blob[i / 8] & (1 << (i % 8))) ? 0xFF : 0x00;
+      rawData[i] = (blob[i / 8] & (1 << (i % 8))) ? 1 : 0;
     return success();
   }
   // Otherwise, fallback to the default behavior.
diff --git a/mlir/test/Bytecode/i1_roundtrip.mlir b/mlir/test/Bytecode/i1_roundtrip.mlir
new file mode 100644
index 0000000000000..dc2529e62430e
--- /dev/null
+++ b/mlir/test/Bytecode/i1_roundtrip.mlir
@@ -0,0 +1,45 @@
+// RUN: mlir-opt %s -emit-bytecode | mlir-opt | FileCheck %s
+
+// CHECK-LABEL: func.func @test_i1_splat_true
+func.func @test_i1_splat_true() -> tensor<100xi1> {
+// CHECK: arith.constant dense<true> : tensor<100xi1>
+  %0 = arith.constant dense<true> : tensor<100xi1>
+  return %0 : tensor<100xi1>
+}
+
+
+// CHECK-LABEL: func.func @test_i1_splat_false
+func.func @test_i1_splat_false() -> tensor<100xi1> {
+// CHECK: arith.constant dense<false> : tensor<100xi1>
+  %0 = arith.constant dense<false> : tensor<100xi1>
+  return %0 : tensor<100xi1>
+}
+
+
+// CHECK-LABEL: func.func @test_8xi1_splat_true
+func.func @test_8xi1_splat_true() -> tensor<8xi1> {
+// CHECK: arith.constant dense<true> : tensor<8xi1>
+  %0 = arith.constant dense<true> : tensor<8xi1>
+  return %0 : tensor<8xi1>
+}
+
+// CHECK-LABEL: func.func @test_8xi1_splat_false
+func.func @test_8xi1_splat_false() -> tensor<8xi1> {
+// CHECK: arith.constant dense<false> : tensor<8xi1>
+  %0 = arith.constant dense<false> : tensor<8xi1>
+  return %0 : tensor<8xi1>
+}
+
+// CHECK-LABEL: func.func @test_i8_mixed()
+func.func @test_i8_mixed() {
+  // CHECK: arith.constant dense<[true, false, true, false, true, false, true, false]> : tensor<8xi1>
+  %0 = arith.constant dense<[true, false, true, false, true, false, true, false]> : tensor<8xi1>
+  return
+}
+
+// CHECK-LABEL: func.func @test_i9_mixed()
+func.func @test_i9_mixed() {
+  // CHECK: arith.constant dense<[true, false, true, false, true, false, true, false, true]> : tensor<9xi1>
+  %0 = arith.constant dense<[true, false, true, false, true, false, true, false, true]> : tensor<9xi1>
+  return
+}
diff --git a/mlir/test/Bytecode/i1_splat_roundtrip.mlir b/mlir/test/Bytecode/i1_splat_roundtrip.mlir
deleted file mode 100644
index dde6fe61934e6..0000000000000
--- a/mlir/test/Bytecode/i1_splat_roundtrip.mlir
+++ /dev/null
@@ -1,17 +0,0 @@
-// RUN: mlir-opt %s -emit-bytecode | mlir-opt | FileCheck %s
-
-func.func @test_i1_splat_true() -> tensor<100xi1> {
-  %0 = arith.constant dense<true> : tensor<100xi1>
-  return %0 : tensor<100xi1>
-}
-
-// CHECK-LABEL: func.func @test_i1_splat_true
-// CHECK: arith.constant dense<true> : tensor<100xi1>
-
-func.func @test_i1_splat_false() -> tensor<100xi1> {
-  %0 = arith.constant dense<false> : tensor<100xi1>
-  return %0 : tensor<100xi1>
-}
-
-// CHECK-LABEL: func.func @test_i1_splat_false
-// CHECK: arith.constant dense<false> : tensor<100xi1>

``````````

</details>


https://github.com/llvm/llvm-project/pull/182445