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

[Haruki Imai via Phabricator via llvm-commits]

imaihal marked 4 inline comments as done.
imaihal added inline comments.


================
Comment at: mlir/lib/Support/EndianUtilities.cpp:54
+  if (storageBitWidth == 16) {
+    ulittle16_t *inRawDataPos = const_cast<ulittle16_t *>(
+        reinterpret_cast<const ulittle16_t *>(inRawData.begin()));
----------------
mehdi_amini wrote:
> It does not work with `const ulittle16_t *` ?
> 
> If so, can you extract the const_cast outside of the sequence of `if` (and make it a switch?):
> ```
> char *rawDataBegin = const_cast<char *>(inRawData.begin());
> switch (storageBitWidth) {
>   case 16: {
>     ulittle16_t *inRawDataPos =  reinterpret_cast<ulittle16_t *>(rawDataBegin);
>     uint16_t *outDataPos  = reinterpret_cast<uint16_t *>(outRawData.begin());
>     for (unsigned i = 0, e = numElements; i < e; ++i)
>       std::copy_n(inRawDataPos + i, 1, outDataPos + i);
>   }
>   ...
> ```
`const ulittle16_t *` works. Thanks!


================
Comment at: mlir/lib/Support/EndianUtilities.cpp:59
+    for (unsigned i = 0, e = numElements; i < e; ++i)
+      std::copy_n(inRawDataPos + i, 1, outDataPos + i);
+  } else if (storageBitWidth == 32) {
----------------
mehdi_amini wrote:
> It isn't clear to me right now why you need a loop instead of using the second argument of std::copy_n?
> 
> ```
> std::copy_n(inRawDataPos, numElements, outDataPos);
> ```
Sorry, it works.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80695