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

[Scott Linder via llvm-commits]

================
@@ -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())
+    handleAllErrors(MsgPackDoc.readFromBlob(Blob, /*Multi=*/false));
----------------
slinder1 wrote:

It might be worth updating that comment, I agree "the frontend" is a little opaque here! I believe it refers to LLPC: https://github.com/GPUOpen-Drivers/llpc

For example: https://github.com/GPUOpen-Drivers/llpc/blob/1005e28c078e07fc0b83363260811f733edaeafd/lgc/state/PalMetadata.cpp#L123 and https://github.com/GPUOpen-Drivers/llpc/blob/1005e28c078e07fc0b83363260811f733edaeafd/lgc/include/lgc/state/AbiMetadata.h#L636

In general we don't want to actually `assert` for any parseable IR/bitcode input, we would rather exit (somewhat) gracefully with an intelligible error, even when not built with asserts enabled.

I can't seem to find where this method is actually used, though. @trenouf do you know where the `AMDGPUPALMetadata::setFrom.*Blob` methods are called? Should they propagate errors to the caller? Or is it OK to `report_fatal_error` it here?

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