[llvm] [MsgPack] Return an Error instead of bool from readFromBlob (PR #74357)

Emma Pilkington via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 5 06:52:15 PST 2023


================
@@ -63,27 +63,28 @@ void AMDGPUPALMetadata::readFromIR(Module &M) {
   }
 }
 
-// Set PAL metadata from a binary blob from the applicable .note record.
-// Returns false if bad format.  Blob must remain valid for the lifetime of the
-// Metadata.
-bool AMDGPUPALMetadata::setFromBlob(unsigned Type, StringRef Blob) {
+// Set PAL metadata from a binary blob from the applicable .note record. Blob
+// must remain valid for the lifetime of the Metadata.
+void AMDGPUPALMetadata::setFromBlob(unsigned Type, StringRef Blob) {
   BlobType = Type;
-  if (Type == ELF::NT_AMD_PAL_METADATA)
-    return setFromLegacyBlob(Blob);
-  return setFromMsgPackBlob(Blob);
+  if (Type == ELF::NT_AMD_PAL_METADATA) {
+    setFromLegacyBlob(Blob);
+  } else {
+    setFromMsgPackBlob(Blob);
+  }
 }
 
 // Set PAL metadata from legacy (array of key=value pairs) blob.
-bool AMDGPUPALMetadata::setFromLegacyBlob(StringRef Blob) {
+void AMDGPUPALMetadata::setFromLegacyBlob(StringRef Blob) {
   auto Data = reinterpret_cast<const uint32_t *>(Blob.data());
   for (unsigned I = 0; I != Blob.size() / sizeof(uint32_t) / 2; ++I)
     setRegister(Data[I * 2], Data[I * 2 + 1]);
-  return true;
 }
 
 // Set PAL metadata from msgpack blob.
-bool AMDGPUPALMetadata::setFromMsgPackBlob(StringRef Blob) {
-  return MsgPackDoc.readFromBlob(Blob, /*Multi=*/false);
+void AMDGPUPALMetadata::setFromMsgPackBlob(StringRef Blob) {
+  if (!Blob.empty())
----------------
epilk wrote:

It was always failing on an empty `Blob`, its just that the errors were being implicitly ignored here before this patch. That seems fine to me since we just treat it as an empty document and add metadata to it. (See: https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/AMDGPU/amdpal-msgpack-es.ll#L14 )

https://github.com/llvm/llvm-project/pull/74357


More information about the llvm-commits mailing list