From 720ea66859ea214051e65d352f8497bf7734f49c Mon Sep 17 00:00:00 2001 From: foreman Date: Mon, 28 Oct 2019 18:13:35 -0400 Subject: [PATCH] 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 | 5 ++-- rocclr/runtime/device/devkernel.hpp | 3 +-- rocclr/runtime/device/devprogram.cpp | 29 +++++++++++++----------- rocclr/runtime/device/devprogram.hpp | 6 ++--- rocclr/runtime/device/pal/palkernel.cpp | 5 +--- rocclr/runtime/device/rocm/rockernel.cpp | 5 +--- 6 files changed, 24 insertions(+), 29 deletions(-) diff --git a/rocclr/runtime/device/devkernel.cpp b/rocclr/runtime/device/devkernel.cpp index 0ba8aed4aa..5f592fdf2d 100644 --- a/rocclr/runtime/device/devkernel.cpp +++ b/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* printfStr) { - +bool Kernel::GetPrintfStr(std::vector* 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, diff --git a/rocclr/runtime/device/devkernel.hpp b/rocclr/runtime/device/devkernel.hpp index f944584252..91b10fb654 100644 --- a/rocclr/runtime/device/devkernel.hpp +++ b/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* printfStr); + bool GetPrintfStr(std::vector* printfStr); //! Returns the kernel symbol name const std::string& symbolName() const { return symbolName_; } diff --git a/rocclr/runtime/device/devprogram.cpp b/rocclr/runtime/device/devprogram.cpp index 3f06b8689d..e9cd42114f 100644 --- a/rocclr/runtime/device/devprogram.cpp +++ b/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; } diff --git a/rocclr/runtime/device/devprogram.hpp b/rocclr/runtime/device/devprogram.hpp index 5f292a5143..6710f865ad 100644 --- a/rocclr/runtime/device/devprogram.hpp +++ b/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 kernelMetadataMap_; //!< Map of kernel metadata + std::map kernelMetadataMap_; //!< Map of kernel metadata #else CodeObjectMD* metadata_; //!< Runtime metadata #endif @@ -218,7 +218,7 @@ class Program : public amd::HeapObject { std::vector 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 { diff --git a/rocclr/runtime/device/pal/palkernel.cpp b/rocclr/runtime/device/pal/palkernel.cpp index cd615c1161..dc4977bd3d 100644 --- a/rocclr/runtime/device/pal/palkernel.cpp +++ b/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 printfStr; - if (!GetPrintfStr(*programMD, &printfStr)) { + if (!GetPrintfStr(&printfStr)) { return false; } diff --git a/rocclr/runtime/device/rocm/rockernel.cpp b/rocclr/runtime/device/rocm/rockernel.cpp index 8a28accfe4..43cc5532c7 100644 --- a/rocclr/runtime/device/rocm/rockernel.cpp +++ b/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(program())->metadata(); - assert(programMD != nullptr); - std::vector printfStr; - if (!GetPrintfStr(*programMD, &printfStr)) { + if (!GetPrintfStr(&printfStr)) { return false; }