From e94aec09bdeb28fabaf2c4f2023939725a2d199f Mon Sep 17 00:00:00 2001 From: Jason Tang Date: Mon, 6 Sep 2021 17:55:09 -0400 Subject: [PATCH] SWDEV-1 - Some 'delete' clean up Change-Id: I02564f0f0e349375bde1471e9f82df268703367b [ROCm/clr commit: 73967c3b17e819d354c3216b7bda861d921b33f7] --- projects/clr/rocclr/device/devhostcall.cpp | 6 +-- projects/clr/rocclr/device/device.cpp | 46 ++++++------------- projects/clr/rocclr/device/devprogram.cpp | 10 ++-- .../clr/rocclr/device/pal/palcounters.cpp | 9 +--- .../clr/rocclr/device/pal/paldebugmanager.cpp | 8 +--- projects/clr/rocclr/device/pal/palgpuopen.cpp | 8 ++-- projects/clr/rocclr/device/pal/palprogram.cpp | 14 +++--- projects/clr/rocclr/device/pal/palvirtual.cpp | 10 +--- projects/clr/rocclr/device/rocm/rocdevice.cpp | 29 +++--------- .../clr/rocclr/device/rocm/rocprogram.cpp | 10 ++-- .../clr/rocclr/device/rocm/rocvirtual.cpp | 6 +-- projects/clr/rocclr/platform/context.cpp | 9 +--- projects/clr/rocclr/platform/program.cpp | 7 ++- 13 files changed, 49 insertions(+), 123 deletions(-) diff --git a/projects/clr/rocclr/device/devhostcall.cpp b/projects/clr/rocclr/device/devhostcall.cpp index c49ecc0126..bedc8de63e 100644 --- a/projects/clr/rocclr/device/devhostcall.cpp +++ b/projects/clr/rocclr/device/devhostcall.cpp @@ -310,8 +310,7 @@ void HostcallListener::terminate() { #if defined(__clang__) #if __has_feature(address_sanitizer) - if (urilocator) - delete urilocator; + delete urilocator; #endif #endif delete doorbell_; @@ -355,8 +354,7 @@ bool HostcallListener::initialize(const amd::Device &dev) { delete doorbell_; #if defined(__clang__) #if __has_feature(address_sanitizer) - if (urilocator) - delete urilocator; + delete urilocator; #endif #endif return false; diff --git a/projects/clr/rocclr/device/device.cpp b/projects/clr/rocclr/device/device.cpp index d651784d8e..b9723c45f8 100644 --- a/projects/clr/rocclr/device/device.cpp +++ b/projects/clr/rocclr/device/device.cpp @@ -481,22 +481,14 @@ Device::~Device() { delete vaCacheMap_; } - if (vaCacheAccess_) { - delete vaCacheAccess_; - } + delete vaCacheAccess_; if (arena_mem_obj_ != nullptr) { arena_mem_obj_->release(); } - // Destroy device settings - if (settings_ != nullptr) { - delete settings_; - } - - if (info_.extensions_ != nullptr) { - delete[] info_.extensions_; - } + delete settings_; + delete[] info_.extensions_; } bool Device::ValidateComgr() { @@ -789,12 +781,8 @@ ClBinary::ClBinary(const amd::Device& dev, BinaryImageFormat bifVer) ClBinary::~ClBinary() { release(); - if (elfIn_) { - delete elfIn_; - } - if (elfOut_) { - delete elfOut_; - } + delete elfIn_; + delete elfOut_; } bool ClBinary::setElfTarget() { @@ -1072,10 +1060,8 @@ bool ClBinary::setElfIn() { } elfIn_ = new amd::Elf(ELFCLASSNONE, binary_, size_, nullptr, amd::Elf::ELF_C_READ); if ((elfIn_ == nullptr) || !elfIn_->isSuccessful()) { - if (elfIn_) { - delete elfIn_; - elfIn_ = nullptr; - } + delete elfIn_; + elfIn_ = nullptr; LogError("Creating input ELF object failed"); return false; } @@ -1084,20 +1070,16 @@ bool ClBinary::setElfIn() { } void ClBinary::resetElfIn() { - if (elfIn_) { - delete elfIn_; - elfIn_ = nullptr; - } + delete elfIn_; + elfIn_ = nullptr; } bool ClBinary::setElfOut(unsigned char eclass, const char* outFile, bool tempFile) { elfOut_ = new amd::Elf(eclass, nullptr, 0, outFile, amd::Elf::ELF_C_WRITE); if ((elfOut_ == nullptr) || !elfOut_->isSuccessful()) { - if (elfOut_) { - delete elfOut_; - elfOut_ = nullptr; - } + delete elfOut_; + elfOut_ = nullptr; LogError("Creating ouput ELF object failed"); return false; } @@ -1109,10 +1091,8 @@ bool ClBinary::setElfOut(unsigned char eclass, } void ClBinary::resetElfOut() { - if (elfOut_) { - delete elfOut_; - elfOut_ = nullptr; - } + delete elfOut_; + elfOut_ = nullptr; } bool ClBinary::loadLlvmBinary(std::string& llvmBinary, diff --git a/projects/clr/rocclr/device/devprogram.cpp b/projects/clr/rocclr/device/devprogram.cpp index db95cf3bd9..0ba0eaee4c 100644 --- a/projects/clr/rocclr/device/devprogram.cpp +++ b/projects/clr/rocclr/device/devprogram.cpp @@ -1426,10 +1426,8 @@ bool Program::initClBinary() { // ================================================================================================ void Program::releaseClBinary() { - if (clBinary_ != nullptr) { - delete clBinary_; - clBinary_ = nullptr; - } + delete clBinary_; + clBinary_ = nullptr; } // ================================================================================================ @@ -2171,9 +2169,7 @@ bool Program::initClBinary(const char* binaryIn, size_t size, amd::Os::FileDesc if (!isElf(bin)) { // Invalid binary. - if (decryptedBin != nullptr) { - delete[] decryptedBin; - } + delete[] decryptedBin; DevLogError("Bin is not ELF \n"); return false; } diff --git a/projects/clr/rocclr/device/pal/palcounters.cpp b/projects/clr/rocclr/device/pal/palcounters.cpp index 3da038efc5..5da623d5dd 100644 --- a/projects/clr/rocclr/device/pal/palcounters.cpp +++ b/projects/clr/rocclr/device/pal/palcounters.cpp @@ -59,13 +59,8 @@ PalCounterReference::~PalCounterReference() { // so we have to lock just this queue amd::ScopedLock lock(gpu_.execution()); - if (layout_ != nullptr) { - delete layout_; - } - - if (memory_ != nullptr) { - delete memory_; - } + delete layout_; + delete memory_; if (nullptr != iPerf()) { iPerf()->Destroy(); diff --git a/projects/clr/rocclr/device/pal/paldebugmanager.cpp b/projects/clr/rocclr/device/pal/paldebugmanager.cpp index 2d0f137c31..7c00abac9e 100644 --- a/projects/clr/rocclr/device/pal/paldebugmanager.cpp +++ b/projects/clr/rocclr/device/pal/paldebugmanager.cpp @@ -64,9 +64,7 @@ GpuDebugManager::GpuDebugManager(amd::Device* device) } GpuDebugManager::~GpuDebugManager() { - if (nullptr != addressWatch_) { - delete[] addressWatch_; - } + delete[] addressWatch_; } void GpuDebugManager::executePreDispatchCallBack(void* aqlPacket, void* toolInfo) { @@ -269,9 +267,7 @@ void GpuDebugManager::setAddressWatch(uint32_t numWatchPoints, void** watchAddre // previously allocated size is not big enough, allocate new memory if (addressWatchSize_ < requiredSize) { - if (nullptr != addressWatch_) { // free the smaller address watch storage - delete[] addressWatch_; - } + delete[] addressWatch_; addressWatch_ = new HwDbgAddressWatch[numWatchPoints]; addressWatchSize_ = requiredSize; } diff --git a/projects/clr/rocclr/device/pal/palgpuopen.cpp b/projects/clr/rocclr/device/pal/palgpuopen.cpp index 3862878b28..a03418a843 100644 --- a/projects/clr/rocclr/device/pal/palgpuopen.cpp +++ b/projects/clr/rocclr/device/pal/palgpuopen.cpp @@ -720,11 +720,9 @@ void RgpCaptureMgr::DestroyRGPTracing() { delete user_event_; // Destroy the GPA session - if (trace_.gpa_session_ != nullptr) { - // Util::Destructor(trace_.gpa_session_); - delete trace_.gpa_session_; - trace_.gpa_session_ = nullptr; - } + // Util::Destructor(trace_.gpa_session_); + delete trace_.gpa_session_; + trace_.gpa_session_ = nullptr; memset(&trace_, 0, sizeof(trace_)); } diff --git a/projects/clr/rocclr/device/pal/palprogram.cpp b/projects/clr/rocclr/device/pal/palprogram.cpp index 6fb737d9d7..5d7a5da542 100644 --- a/projects/clr/rocclr/device/pal/palprogram.cpp +++ b/projects/clr/rocclr/device/pal/palprogram.cpp @@ -226,9 +226,9 @@ HSAILProgram::~HSAILProgram() { if (executable_) { loader_->DestroyExecutable(executable_); } - if (kernels_) { - delete kernels_; - } + + delete kernels_; + if (loader_) { amd::hsa::loader::Loader::Destroy(loader_); } @@ -282,16 +282,14 @@ bool HSAILProgram::createKernels(void* binary, size_t binSize, bool useUniformWo return false; } if (kernelNamesSize > 0) { - char* kernelNames = new char[kernelNamesSize]; + std::vector kernelNames(kernelNamesSize); errorCode = amd::Hsail::QueryInfo(palNullDevice().compiler(), binaryElf_, RT_KERNEL_NAMES, - nullptr, kernelNames, &kernelNamesSize); + nullptr, kernelNames.data(), &kernelNamesSize); if (errorCode != ACL_SUCCESS) { buildLog_ += "Error: Querying of kernel names from the binary failed.\n"; - delete[] kernelNames; return false; } - std::vector vKernels = splitSpaceSeparatedString(kernelNames); - delete[] kernelNames; + std::vector vKernels = splitSpaceSeparatedString(kernelNames.data()); for (const auto& it : vKernels) { std::string kernelName(it); diff --git a/projects/clr/rocclr/device/pal/palvirtual.cpp b/projects/clr/rocclr/device/pal/palvirtual.cpp index 0238012549..355fc5df5c 100644 --- a/projects/clr/rocclr/device/pal/palvirtual.cpp +++ b/projects/clr/rocclr/device/pal/palvirtual.cpp @@ -974,7 +974,6 @@ bool VirtualGPU::create(bool profiling, uint deviceQueueSize, uint rtCUs, // Create HSAILPrintf class printfDbgHSA_ = new PrintfDbgHSA(gpuDevice_); if (nullptr == printfDbgHSA_) { - delete printfDbgHSA_; LogError("Could not create PrintfDbgHSA class!"); return false; } @@ -1107,13 +1106,8 @@ VirtualGPU::~VirtualGPU() { { // Destroy queues - if (nullptr != queues_[MainEngine]) { - delete queues_[MainEngine]; - } - - if (nullptr != queues_[SdmaEngine]) { - delete queues_[SdmaEngine]; - } + delete queues_[MainEngine]; + delete queues_[SdmaEngine]; if (nullptr != cmdAllocator_) { cmdAllocator_->Destroy(); diff --git a/projects/clr/rocclr/device/rocm/rocdevice.cpp b/projects/clr/rocclr/device/rocm/rocdevice.cpp index 77e3430cc0..b4be397737 100644 --- a/projects/clr/rocclr/device/rocm/rocdevice.cpp +++ b/projects/clr/rocclr/device/rocm/rocdevice.cpp @@ -244,15 +244,9 @@ Device::~Device() { context_->release(); } - if (info_.extensions_) { - delete[] info_.extensions_; - info_.extensions_ = nullptr; - } + delete[] info_.extensions_; - if (settings_) { - delete settings_; - settings_ = nullptr; - } + delete settings_; delete[] p2p_agents_list_; @@ -316,15 +310,8 @@ bool NullDevice::init() { } NullDevice::~NullDevice() { - if (info_.extensions_) { - delete[] info_.extensions_; - info_.extensions_ = nullptr; - } - - if (settings_) { - delete settings_; - settings_ = nullptr; - } + delete[] info_.extensions_; + delete settings_; } hsa_status_t Device::iterateAgentCallback(hsa_agent_t agent, void* data) { @@ -2914,13 +2901,12 @@ bool Device::findLinkInfo(const hsa_amd_memory_pool_t& pool, } // Retrieve link info on the pool. - hsa_amd_memory_pool_link_info_t* link_info = new hsa_amd_memory_pool_link_info_t[hops]; + std::vector link_info(hops); hsa_status = hsa_amd_agent_memory_pool_get_info(_bkendDevice, pool, - HSA_AMD_AGENT_MEMORY_POOL_INFO_LINK_INFO, link_info); + HSA_AMD_AGENT_MEMORY_POOL_INFO_LINK_INFO, link_info.data()); if (hsa_status != HSA_STATUS_SUCCESS) { DevLogPrintfError("Cannot retrieve link info, hsa failed with status: %d", hsa_status); - delete[] link_info; return false; } @@ -2959,14 +2945,11 @@ bool Device::findLinkInfo(const hsa_amd_memory_pool_t& pool, } default: { DevLogPrintfError("Invalid LinkAttribute: %d ", link_attr.first); - delete[] link_info; return false; } } } - delete[] link_info; - return true; } diff --git a/projects/clr/rocclr/device/rocm/rocprogram.cpp b/projects/clr/rocclr/device/rocm/rocprogram.cpp index 32aed3d55d..83bae886c4 100644 --- a/projects/clr/rocclr/device/rocm/rocprogram.cpp +++ b/projects/clr/rocclr/device/rocm/rocprogram.cpp @@ -73,8 +73,7 @@ bool Program::initClBinary(char* binaryIn, size_t size) { char* decryptedBin; size_t decryptedSize; if (!clBinary()->decryptElf(binaryIn, size, &decryptedBin, &decryptedSize, &encryptCode)) { - buildLog_ += "Decrypting ELF Failed "; - buildLog_ += "\n"; + buildLog_ += "Decrypting ELF Failed\n"; return false; } if (decryptedBin != nullptr) { @@ -86,11 +85,8 @@ bool Program::initClBinary(char* binaryIn, size_t size) { // Both 32-bit and 64-bit are allowed! if (!amd::Elf::isElfMagic(bin)) { // Invalid binary. - if (decryptedBin != nullptr) { - delete[] decryptedBin; - } - buildLog_ += "Elf Magic failed"; - buildLog_ += "\n"; + delete[] decryptedBin; + buildLog_ += "Elf Magic failed\n"; return false; } diff --git a/projects/clr/rocclr/device/rocm/rocvirtual.cpp b/projects/clr/rocclr/device/rocm/rocvirtual.cpp index f616c2fd02..8123cd3859 100644 --- a/projects/clr/rocclr/device/rocm/rocvirtual.cpp +++ b/projects/clr/rocclr/device/rocm/rocvirtual.cpp @@ -1086,10 +1086,8 @@ VirtualGPU::~VirtualGPU() { timestamp_ = nullptr; LogError("There was a timestamp that was not used; deleting."); } - if (printfdbg_ != nullptr) { - delete printfdbg_; - printfdbg_ = nullptr; - } + + delete printfdbg_; if (0 != schedulerSignal_.handle) { hsa_signal_destroy(schedulerSignal_); diff --git a/projects/clr/rocclr/platform/context.cpp b/projects/clr/rocclr/platform/context.cpp index 2901f7c9bd..27450cb213 100644 --- a/projects/clr/rocclr/platform/context.cpp +++ b/projects/clr/rocclr/platform/context.cpp @@ -89,13 +89,8 @@ Context::~Context() { it->release(); } - if (properties_ != NULL) { - delete[] properties_; - } - if (glenv_ != NULL) { - delete glenv_; - glenv_ = NULL; - } + delete[] properties_; + delete glenv_; #if WITH_LIQUID_FLASH lfTerminate(); diff --git a/projects/clr/rocclr/platform/program.cpp b/projects/clr/rocclr/platform/program.cpp index 98900e8a17..663e4b6289 100644 --- a/projects/clr/rocclr/platform/program.cpp +++ b/projects/clr/rocclr/platform/program.cpp @@ -467,12 +467,11 @@ void Program::StubProgramSource(const std::string& app_name) { size_t size = stub_read.tellg(); stub_read.seekg(0, std::ios::beg); - char* data = new char[size]; - stub_read.read(data, size); + std::vector file_data(size); + stub_read.read(file_data.data(), size); stub_read.close(); - sourceCode_.assign(data, size); - delete[] data; + sourceCode_.assign(file_data.data(), size); } else { std::fstream stub_write; stub_write.open(file_name.str().c_str(), (std::fstream::out | std::fstream::binary));