From 8047d8e3e899970aad094e2c8daba8a1429ebcf8 Mon Sep 17 00:00:00 2001 From: Jacob Lambert Date: Fri, 9 Jun 2023 13:33:57 -0400 Subject: [PATCH] Revert "SWDEV-371628 - Shift device lib linking into clang driver" This reverts commit a4c5c440085a911d5802222578d85feeed8bef90. Reason for revert: Performance regressions and failures observed. Need to investigate those and before re-applying patch Change-Id: I42ba0605797f9bdcfb5d5102927dd01405cf05e3 --- rocclr/device/devprogram.cpp | 46 ++++++++++++++++++++++-------------- rocclr/device/devprogram.hpp | 6 ++--- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/rocclr/device/devprogram.cpp b/rocclr/device/devprogram.cpp index f989f13ef4..493f20b48d 100644 --- a/rocclr/device/devprogram.cpp +++ b/rocclr/device/devprogram.cpp @@ -348,7 +348,7 @@ amd_comgr_status_t Program::createAction(const amd_comgr_language_t oclver, bool Program::linkLLVMBitcode(const amd_comgr_data_set_t inputs, const std::vector& options, amd::option::Options* amdOptions, amd_comgr_data_set_t* output, - char* binaryData[], size_t* binarySize) { + char* binaryData[], size_t* binarySize, const bool link_dev_libs) { amd_comgr_language_t langver = getCOMGRLanguage(isHIP(), *amdOptions); if (langver == AMD_COMGR_LANGUAGE_NONE) { @@ -357,13 +357,28 @@ bool Program::linkLLVMBitcode(const amd_comgr_data_set_t inputs, // Create the action for linking amd_comgr_action_info_t action; + amd_comgr_data_set_t dataSetDevLibs; bool hasAction = false; + bool hasDataSetDevLibs = false; amd_comgr_status_t status = createAction(langver, options, &action, &hasAction); + if (link_dev_libs) { + if (status == AMD_COMGR_STATUS_SUCCESS) { + status = amd::Comgr::create_data_set(&dataSetDevLibs); + } + + if (status == AMD_COMGR_STATUS_SUCCESS) { + hasDataSetDevLibs = true; + status = amd::Comgr::do_action(AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES, action, inputs, + dataSetDevLibs); + extractBuildLog(dataSetDevLibs); + } + } + if (status == AMD_COMGR_STATUS_SUCCESS) { status = amd::Comgr::do_action(AMD_COMGR_ACTION_LINK_BC_TO_BC, action, - inputs, *output); + (link_dev_libs) ? dataSetDevLibs : inputs, *output); extractBuildLog(*output); } @@ -380,14 +395,17 @@ bool Program::linkLLVMBitcode(const amd_comgr_data_set_t inputs, amd::Comgr::destroy_action_info(action); } + if (hasDataSetDevLibs) { + amd::Comgr::destroy_data_set(dataSetDevLibs); + } + return (status == AMD_COMGR_STATUS_SUCCESS); } bool Program::compileToLLVMBitcode(const amd_comgr_data_set_t compileInputs, const std::vector& options, amd::option::Options* amdOptions, - char* binaryData[], size_t* binarySize, - const bool link_dev_libs) { + char* binaryData[], size_t* binarySize) { amd_comgr_language_t langver = getCOMGRLanguage(isHIP(), *amdOptions); if (langver == AMD_COMGR_LANGUAGE_NONE) { @@ -461,14 +479,8 @@ bool Program::compileToLLVMBitcode(const amd_comgr_data_set_t compileInputs, // Compiling the source codes with precompiled headers or directly compileInputs if (status == AMD_COMGR_STATUS_SUCCESS) { - if (link_dev_libs) { - status = amd::Comgr::do_action(AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC, - action, input, output); - } - else { - status = amd::Comgr::do_action(AMD_COMGR_ACTION_COMPILE_SOURCE_TO_BC, - action, input, output); - } + status = amd::Comgr::do_action(AMD_COMGR_ACTION_COMPILE_SOURCE_TO_BC, + action, input, output); extractBuildLog(output); } @@ -665,11 +677,8 @@ bool Program::compileImplLC(const std::string& sourceCode, driverOptions.push_back("-mllvm"); driverOptions.push_back("-amdgpu-early-inline-all"); #endif - - // TODO: Re-enable prelink once folding optimizations no longer - // introduce undefined symbol errors - //driverOptions.push_back("-mllvm"); - //driverOptions.push_back("-amdgpu-prelink"); + driverOptions.push_back("-mllvm"); + driverOptions.push_back("-amdgpu-prelink"); if (!device().settings().enableWgpMode_) { driverOptions.push_back("-mcumode"); @@ -959,8 +968,9 @@ bool Program::linkImplLC(const std::vector& inputPrograms, char* binaryData = nullptr; size_t binarySize = 0; std::vector linkOptions; + constexpr bool kLinkDevLibs = false; bool ret = linkLLVMBitcode(inputs, linkOptions, options, &output, &binaryData, - &binarySize); + &binarySize, kLinkDevLibs); amd::Comgr::destroy_data_set(output); amd::Comgr::destroy_data_set(inputs); diff --git a/rocclr/device/devprogram.hpp b/rocclr/device/devprogram.hpp index 08a8d0dec2..55eac9984e 100644 --- a/rocclr/device/devprogram.hpp +++ b/rocclr/device/devprogram.hpp @@ -457,13 +457,13 @@ class Program : public amd::HeapObject { bool linkLLVMBitcode(const amd_comgr_data_set_t inputs, const std::vector& options, amd::option::Options* amdOptions, amd_comgr_data_set_t* output, - char* binaryData[] = nullptr, size_t* binarySize = nullptr); + char* binaryData[] = nullptr, size_t* binarySize = nullptr, + const bool link_dev_libs = true); //! Create the bitcode of the compiled input dataset bool compileToLLVMBitcode(const amd_comgr_data_set_t compileInputs, const std::vector& options, amd::option::Options* amdOptions, - char* binaryData[], size_t* binarySize, - const bool link_dev_libs = true); + char* binaryData[], size_t* binarySize); //! Compile and create the excutable of the input dataset bool compileAndLinkExecutable(const amd_comgr_data_set_t inputs,