[PATCH] D57028: [AMDGPU] Added MsgPack format PAL metadata

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.
Herald added a project: LLVM.


================
Comment at: lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp:51-56
+  std::string Blob;
+  toMsgPackBlob(Blob);
+  auto MDS = MDString::get(M->getContext(), Blob);
+  auto Node = MDNode::get(M->getContext(), MDS);
+  auto Named = M->getOrInsertNamedMetadata("amdgpu.pal.metadata.msgpack");
+  Named->addOperand(Node);
----------------
After thinking more about this and talking to @t-tye and others, I think it might be worth considering other ways to convey metadata through IR. We might also be able to avoid the issue of the front-end needing to make calls into the back-end if we just agree on a scheme and document it; if there is a contract between the front-end and back-end then you can safely put all of the code in the AMDGPU back-end without the private-headers issue.

I'm not sure I like the non-human-readable-blob approach. Since we have the YAML-Msgpack interop could this be a YAML string? I think this is something we would like to incorporate for HSA as well, and one requirement would be that it is readable and editable in IR.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57028