[PATCH] D80695: [mlir] Convert raw data in dense element attributes for big-endian machines.

[Haruki Imai via Phabricator via llvm-commits]

imaihal added a comment.

@rriddle Thanks for your comments. I answered some of the comments.



================
Comment at: mlir/include/mlir/IR/Attributes.h:1143
 
+  /// Convert endianess of input ArrayRef for big-endian(BE) machines. All of
+  /// the elements of `inRawData` has `type`. If `inRawData` is little endian
----------------
rriddle wrote:
> Should we even support calling this if the machine is already LE?
We don't have to call this function in LE machines, but we can call this even in LE machines. 
LE machine:
`inRawData` LE -> `outRawData` LE
BE machine:
 `inRawData` LE -> `outRawData` BE

As you wrote before, there are redundant copy when we use this function in LE machine. To avoid this redundant copy, I think we need to check machine endianness as in this patch. 


================
Comment at: mlir/lib/IR/AsmPrinter.cpp:1517
+      MutableArrayRef<char> convRawData(outDataVec);
+      DenseIntOrFPElementsAttr::convertEndianOfArrayRefForBEmachine(
+          rawData, convRawData, type);
----------------
rriddle wrote:
> I may be confused right now, but doesn't `convertEndianOfArrayRefForBEmachine` just convert from little to big and not big to little?
This may be confusing, but this also converts big to little on BE machine.

`copy_n` copies `inRawData`(`ulittle`) to `outRawData`(`uint`).  This copy assumes `inRawData` is LE format. So, this copy_n always converts endianness on BE machine even in actual `inRawData` is BE format.

Normally this is used when `inRawData` is LE format, but I reused to convert BE to LE.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80695/new/

https://reviews.llvm.org/D80695