[mlir] Fix endian conversion of sub-byte types

[uweigand]

Summary

As noticed in https://reviews.llvm.org/D122925, the routine DenseIntOrFPElementsAttr::convertEndianOfArrayRefForBEmachine does not handle sub-byte types correctly.

convertEndianOfArrayRefForBEmachine may call convertEndianOfCharForBEmachine with an elementBitWidth of 1, which falls into this default case:

default: {
  size_t nBytes = elementBitWidth / CHAR_BIT;
  for (size_t i = 0; i < nBytes; i++)
    std::copy_n(inRawData + (nBytes - 1 - i), 1, outRawData + i);
  break;
}

If elementBitWidth is smaller than CHAR_BIT, that simply does nothing and leaves outRawData unmodified.

However, for sub-byte types no endian conversion is actually necessary, so this patch simply copies the input to the output.

Note that this touches only one caller of DenseIntOrFPElementsAttr::convertEndianOfArrayRefForBEmachine, but all other callers always pass a multiple of CHAR_BITS as elementBitWidth.

[yaochengji]

Thanks @uweigand for fixing this.

And I also found that mlir lacks i1 dtype test of hex format. Maybe we should complete these tests to find out whether more bugs are triggered.

[uweigand]

Ping? The mlir build bot on s390x is still red because of this problem.

[jpienaar]

Does this also show with an i4 test case? (i1 is special in a few ways, so having other sub CHAR_BIT test case would be good)

[uweigand]

Add test case.

[uweigand]

In D125676#3563470, @jpienaar wrote:

Does this also show with an i4 test case? (i1 is special in a few ways, so having other sub CHAR_BIT test case would be good)

Yes, it does. I've now added a bunch of tests, and without the fix, the i1, i4, and i8 tests fail for me . (The i16, i32, i64 tests pass either way.)

mlir/lib/IR/BuiltinAttributes.cpp

void DenseIntOrFPElementsAttr::convertEndianOfArrayRefForBEmachine(
   Type elementType = type.getElementType();
   if (ComplexType complexTy = elementType.dyn_cast<ComplexType>()) {
     elementType = complexTy.getElementType();
     numElements = numElements * 2;
   }
   size_t elementBitWidth = getDenseElementStorageWidth(elementType);
   assert(numElements * elementBitWidth == inRawData.size() * CHAR_BIT &&
          inRawData.size() <= outRawData.size());
+  if (elementBitWidth <= CHAR_BIT)
+    std::memcpy(outRawData.begin(), inRawData.begin(), inRawData.size());
+  else
     convertEndianOfCharForBEmachine(inRawData.begin(), outRawData.begin(),
                                     elementBitWidth, numElements);
 }
 
 //===----------------------------------------------------------------------===//
 // DenseFPElementsAttr
 //===----------------------------------------------------------------------===//
 
 template <typename Fn, typename Attr>
 static ShapedType mappingHelper(Fn mapping, Attr &attr, ShapedType inType,

mlir/test/IR/parse-literal.mlir

+// RUN: mlir-opt %s | FileCheck %s
+
+// CHECK-LABEL: @parse_i64_tensor
+func.func @parse_i64_tensor() -> tensor<4xi64> {
+  // CHECK: dense<255> : tensor<4xi64>
+  %0 = arith.constant dense<"0xFF00000000000000FF00000000000000FF00000000000000FF00000000000000"> : tensor<4xi64>
+  return %0 : tensor<4xi64>
+}
+
+// CHECK-LABEL: @parse_i32_tensor
+func.func @parse_i32_tensor() -> tensor<8xi32> {
+  // CHECK: dense<255> : tensor<8xi32>
+  %0 = arith.constant dense<"0xFF000000FF000000FF000000FF000000FF000000FF000000FF000000FF000000"> : tensor<8xi32>
+  return %0 : tensor<8xi32>
+}
+
+// CHECK-LABEL: @parse_i16_tensor
+func.func @parse_i16_tensor() -> tensor<16xi16> {
+  // CHECK: dense<255> : tensor<16xi16>
+  %0 = arith.constant dense<"0xFF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00"> : tensor<16xi16>
+  return %0 : tensor<16xi16>
+}
+
+// CHECK-LABEL: @parse_i8_tensor
+func.func @parse_i8_tensor() -> tensor<32xi8> {
+  // CHECK: dense<15> : tensor<32xi8>
+  %0 = arith.constant dense<"0x0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F"> : tensor<32xi8>
+  return %0 : tensor<32xi8>
+}
+
+// CHECK-LABEL: @parse_i4_tensor
+func.func @parse_i4_tensor() -> tensor<32xi4> {
+  // CHECK: dense<-1> : tensor<32xi4>
+  %0 = arith.constant dense<"0x0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F"> : tensor<32xi4>
+  return %0 : tensor<32xi4>
+}
+
+// CHECK-LABEL: @parse_i1_tensor
+func.func @parse_i1_tensor() -> tensor<256xi1> {
+  // CHECK: dense<"0x0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F"> : tensor<256xi1>
+  %0 = arith.constant dense<"0x0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F"> : tensor<256xi1>
+  return %0 : tensor<256xi1>
+}