P4 to Git Change 2020678 by gandryey@gera-win10 on 2019/10/28 18:06:48

SWDEV-79445 - OCL generic changes and code clean-up
	- Fix memory leaks in COMGR path. Make sure  metadata_ object is properly destroyed in COMGR.

Affected files ...

... //depot/stg/opencl/drivers/opencl/runtime/device/devkernel.cpp#28 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/devkernel.hpp#18 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/devprogram.cpp#66 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/devprogram.hpp#34 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palkernel.cpp#84 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/rocm/rockernel.cpp#53 edit

rocclr/runtime/device/devkernel.cpp

@@ -1397,9 +1397,8 @@ bool Kernel::SetAvailableSgprVgpr(const std::string& targetIdent) {
   return (status == AMD_COMGR_STATUS_SUCCESS);
 }
 
-bool Kernel::GetPrintfStr(const amd_comgr_metadata_node_t programMD,
-                          std::vector<std::string>* printfStr) {
-
+bool Kernel::GetPrintfStr(std::vector<std::string>* printfStr) {
+  const amd_comgr_metadata_node_t programMD = prog().metadata();
   amd_comgr_metadata_node_t printfMeta;
 
   amd_comgr_status_t status = amd::Comgr::metadata_lookup(programMD,

rocclr/runtime/device/devkernel.hpp

@@ -506,8 +506,7 @@ class Kernel : public amd::HeapObject {
   bool SetAvailableSgprVgpr(const std::string& targetIdent);
 
   //! Retrieve the printf string metadata
-  bool GetPrintfStr(const amd_comgr_metadata_node_t programMD,
-                    std::vector<std::string>* printfStr);
+  bool GetPrintfStr(std::vector<std::string>* printfStr);
 
   //! Returns the kernel symbol name
   const std::string& symbolName() const { return symbolName_; }

rocclr/runtime/device/devprogram.cpp

@@ -82,7 +82,7 @@ Program::Program(amd::Device& device, amd::Program& owner)
       machineTarget_(nullptr),
       globalVariableTotalSize_(0),
       programOptions_(nullptr),
-      metadata_(nullptr)
+      metadata_{0}
 {
   memset(&binOpts_, 0, sizeof(binOpts_));
   binOpts_.struct_size = sizeof(binOpts_);
@@ -101,12 +101,16 @@ Program::~Program() {
     (*it)->release();
   }
 
+  if (isLC()) {
 #if defined(USE_COMGR_LIBRARY)
-  for (auto const& kernelMeta : kernelMetadataMap_) {
-    amd::Comgr::destroy_metadata(kernelMeta.second);
-  }
+    for (auto const& kernelMeta : kernelMetadataMap_) {
+      amd::Comgr::destroy_metadata(kernelMeta.second);
+    }
+    amd::Comgr::destroy_metadata(metadata_);
+#else
+    delete metadata_;
 #endif
-  delete metadata_;
+  }
 }
 
 // ================================================================================================
@@ -2898,14 +2902,14 @@ bool Program::createKernelMetadataMap() {
   bool hasKernelMD = false;
   size_t size = 0;
 
-  status = amd::Comgr::metadata_lookup(*metadata_, "Kernels", &kernelsMD);
+  status = amd::Comgr::metadata_lookup(metadata_, "Kernels", &kernelsMD);
   if (status == AMD_COMGR_STATUS_SUCCESS) {
     LogInfo("Using Code Object V2.");
     hasKernelMD = true;
     codeObjectVer_ = 2;
   }
   else {
-    status = amd::Comgr::metadata_lookup(*metadata_, "amdhsa.kernels", &kernelsMD);
+    status = amd::Comgr::metadata_lookup(metadata_, "amdhsa.kernels", &kernelsMD);
 
     if (status == AMD_COMGR_STATUS_SUCCESS) {
       LogInfo("Using Code Object V3.");
@@ -2987,7 +2991,7 @@ bool Program::FindGlobalVarSize(void* binary, size_t binSize) {
     buildLog_ += "Error while reading the ELF program binary\n";
     return false;
   }
-
+  bool metadata_found = false;
   for (size_t i = 0; i < numpHdrs; ++i) {
     GElf_Phdr pHdr;
     if (gelf_getphdr(e, i, &pHdr) != &pHdr) {
@@ -3024,8 +3028,7 @@ bool Program::FindGlobalVarSize(void* binary, size_t binSize) {
           }
 
           if (status == AMD_COMGR_STATUS_SUCCESS) {
-            metadata_ = new amd_comgr_metadata_node_t;
-            status = amd::Comgr::get_data_metadata(binaryData, metadata_);
+            status = amd::Comgr::get_data_metadata(binaryData, &metadata_);
           }
 
           amd::Comgr::release_data(binaryData);
@@ -3044,6 +3047,7 @@ bool Program::FindGlobalVarSize(void* binary, size_t binSize) {
           // We've found and loaded the runtime metadata, exit the
           // note record loop now.
 #endif
+          metadata_found = true;
           break;
         }
         ptr += sizeof(*note) + amd::alignUp(note->n_namesz, sizeof(int)) +
@@ -3066,9 +3070,8 @@ bool Program::FindGlobalVarSize(void* binary, size_t binSize) {
 
   elf_end(e);
 
-  if (!metadata_) {
-    buildLog_ +=
-      "Error: runtime metadata section not present in ELF program binary\n";
+  if (!metadata_found) {
+    buildLog_ += "Error: runtime metadata section not present in ELF program binary\n";
     return false;
   }
 

rocclr/runtime/device/devprogram.hpp

@@ -124,9 +124,9 @@ class Program : public amd::HeapObject {
 
 
 #if defined(USE_COMGR_LIBRARY)
-  amd_comgr_metadata_node_t* metadata_;   //!< COMgr metadata
+  amd_comgr_metadata_node_t metadata_;    //!< COMgr metadata
   uint32_t codeObjectVer_;                //!< version of code object
-  std::map<std::string,amd_comgr_metadata_node_t> kernelMetadataMap_; //!< Map of kernel metadata
+  std::map<std::string, amd_comgr_metadata_node_t> kernelMetadataMap_; //!< Map of kernel metadata
 #else
   CodeObjectMD* metadata_;  //!< Runtime metadata
 #endif
@@ -218,7 +218,7 @@ class Program : public amd::HeapObject {
   std::vector<amd::Memory*> getUndefMemObj() const { return undef_mem_obj_; }
 
 #if defined(USE_COMGR_LIBRARY)
-  const amd_comgr_metadata_node_t* metadata() const { return metadata_; }
+  amd_comgr_metadata_node_t metadata() const { return metadata_; }
 
   //! Get the kernel metadata
   const amd_comgr_metadata_node_t* getKernelMetadata(const std::string name) const {

rocclr/runtime/device/pal/palkernel.cpp

@@ -495,11 +495,8 @@ bool LightningKernel::init() {
   }
 
   // handle the printf metadata if any
-  const amd_comgr_metadata_node_t* programMD = prog().metadata();
-  assert(programMD != nullptr);
-
   std::vector<std::string> printfStr;
-  if (!GetPrintfStr(*programMD, &printfStr)) {
+  if (!GetPrintfStr(&printfStr)) {
     return false;
   }
 

rocclr/runtime/device/rocm/rockernel.cpp

@@ -160,11 +160,8 @@ bool LightningKernel::init() {
   }
 
   // handle the printf metadata if any
-  const amd_comgr_metadata_node_t* programMD = static_cast<const LightningProgram*>(program())->metadata();
-  assert(programMD != nullptr);
-
   std::vector<std::string> printfStr;
-  if (!GetPrintfStr(*programMD, &printfStr)) {
+  if (!GetPrintfStr(&printfStr)) {
     return false;
   }