[PATCH] D57027: [AMDGPU] Factored PAL metadata handling out into its own class

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp:170-177
+  Stream << "\t" << AMDGPU::PALMD::AssemblerDirective << " ";
+  for (auto I = Registers.begin(), E = Registers.end(); I != E; ++I) {
+    if (I != Registers.begin())
+      Stream << ",";
+    Stream << "0x" << Twine::utohexstr(I->first) << ",0x"
+           << Twine::utohexstr(I->second);
+  }
----------------
Single quotes around the single characters


================
Comment at: lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp:189-195
+    union {
+      char AsBytes[8];
+      uint32_t AsInts[2];
+    } U;
+    U.AsInts[0] = I.first;
+    U.AsInts[1] = I.second;
+    Blob.insert(Blob.size(), &U.AsBytes[0], 8);
----------------
scott.linder wrote:
> I think this kind of pun is UB in C++, and this doesn't handle big-endian hosts. A `memcpy` would work, but would still require handling endianness. I think just masking out each individual byte handles everything.
Three's an endianness writer in support


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57027