[PATCH] D85667: Reset PAL metadata when AMDGPU traget stream finishes

[Steven Perron via Phabricator via llvm-commits]

s-perron created this revision.
s-perron added a reviewer: nhaehnle.
Herald added subscribers: llvm-commits, kerbowa, hiraditya, t-tye, tpr, dstuttard, yaxunl, jvesely, kzhuravl, arsenm.
Herald added a project: LLVM.
s-perron requested review of this revision.
Herald added a subscriber: wdng.

If the same stream object is used for multiple compiles, the PAL metadata from eariler compilations will leak into later one.  See https://github.com/GPUOpen-Drivers/llpc/issues/882 for how this is happening in LLPC.

No tests were added because multiple compiles will have to happen using the same pass manager, and I do not see a setup for that on the LLVM side.  Let me know if there is a good way to test this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85667

Files:
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.h


Index: llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.h
===================================================================
--- llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.h
+++ llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.h
@@ -106,6 +106,9 @@
   // Set legacy PAL metadata format.
   void setLegacy();
 
+  // Erase all PAL metadata.
+  void reset();
+
 private:
   // Return whether the blob type is legacy PAL metadata.
   bool isLegacy() const;
Index: llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
+++ llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
@@ -773,3 +773,9 @@
   BlobType = ELF::NT_AMD_AMDGPU_PAL_METADATA;
 }
 
+// Erase all PAL metadata.
+void AMDGPUPALMetadata::reset() {
+  MsgPackDoc.clear();
+  Registers = MsgPackDoc.getEmptyNode();
+  HwStages = MsgPackDoc.getEmptyNode();
+}
Index: llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp
+++ llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp
@@ -166,10 +166,15 @@
 
 // A hook for emitting stuff at the end.
 // We use it for emitting the accumulated PAL metadata as directives.
+// The PAL metadata is reset after it is emitted.
 void AMDGPUTargetAsmStreamer::finish() {
   std::string S;
   getPALMetadata()->toString(S);
   OS << S;
+
+  // Reset the pal metadata so is data will not affect a compilation that reuses
+  // this object.
+  getPALMetadata()->reset();
 }
 
 void AMDGPUTargetAsmStreamer::EmitDirectiveAMDGCNTarget(StringRef Target) {
@@ -421,6 +426,7 @@
 
 // A hook for emitting stuff at the end.
 // We use it for emitting the accumulated PAL metadata as a .note record.
+// The PAL metadata is reset after it is emitted.
 void AMDGPUTargetELFStreamer::finish() {
   std::string Blob;
   const char *Vendor = getPALMetadata()->getVendor();
@@ -430,6 +436,10 @@
     return;
   EmitNote(Vendor, MCConstantExpr::create(Blob.size(), getContext()), Type,
            [&](MCELFStreamer &OS) { OS.emitBytes(Blob); });
+
+  // Reset the pal metadata so is data will not affect a compilation that reuses
+  // this object.
+  getPALMetadata()->reset();
 }
 
 void AMDGPUTargetELFStreamer::EmitNote(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85667.284429.patch
Type: text/x-patch
Size: 2369 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200810/567afe2b/attachment.bin>