From f3dc04a50d13c54926e80bba7ad5483dd798672a Mon Sep 17 00:00:00 2001 From: Juan Manuel Martinez Date: Tue, 29 Aug 2023 16:17:22 +0200 Subject: [PATCH] SWDEV-410182: Link device-libs when compiling source to bitcode This is related to SWDEV-410182, but it's not enough to fix it. Functions from device-libs are precompiled into llvm-ir in a "target agnostic" way (in reality, it's not 100% target agnostic, which brings us many headaches). When linking builtins (like device-libs) from the command line, we use the flag -mlink-builtin-bitcode. The difference between regular linking of bitcode and this flag is that the later propagates target-specific attributes. If this attributes are not propagated, we can end up with incosistent target attributes. Comgr provides the action AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC for this exact reason. The old action is currently deprecated and this one should be used. Change-Id: I518415214debdf4fedf0b1d81456d6e9fb8a3d19 --- rocclr/device/devprogram.cpp | 90 ++++-------------------------------- rocclr/device/devprogram.hpp | 3 +- 2 files changed, 11 insertions(+), 82 deletions(-) diff --git a/rocclr/device/devprogram.cpp b/rocclr/device/devprogram.cpp index 493f20b48d..6b41ea75ce 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, const bool link_dev_libs) { + char* binaryData[], size_t* binarySize) { amd_comgr_language_t langver = getCOMGRLanguage(isHIP(), *amdOptions); if (langver == AMD_COMGR_LANGUAGE_NONE) { @@ -357,28 +357,12 @@ 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, - (link_dev_libs) ? dataSetDevLibs : inputs, *output); + status = amd::Comgr::do_action(AMD_COMGR_ACTION_LINK_BC_TO_BC, action, inputs, *output); extractBuildLog(*output); } @@ -395,10 +379,6 @@ 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); } @@ -479,7 +459,7 @@ 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) { - status = amd::Comgr::do_action(AMD_COMGR_ACTION_COMPILE_SOURCE_TO_BC, + status = amd::Comgr::do_action(AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC, action, input, output); extractBuildLog(output); } @@ -968,9 +948,8 @@ 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, kLinkDevLibs); + &binarySize); amd::Comgr::destroy_data_set(output); amd::Comgr::destroy_data_set(inputs); @@ -1140,14 +1119,19 @@ bool Program::linkImplLC(amd::option::Options* options) { return false; } - bool bLinkLLVMBitcode = true; if (llvmBinary_.empty()) { continueCompileFrom = getNextCompilationStageFromBinary(options); } + switch (continueCompileFrom) { case FILE_TYPE_CG: case FILE_TYPE_LLVMIR_BINARY: { + if(addCodeObjData(llvmBinary_.data(), llvmBinary_.size(), + AMD_COMGR_DATA_KIND_BC, + "LLVM Binary.bc", &inputs) != AMD_COMGR_STATUS_SUCCESS) { + return false; + } break; } case FILE_TYPE_ASM_TEXT: { @@ -1162,7 +1146,6 @@ bool Program::linkImplLC(amd::option::Options* options) { return false; } - bLinkLLVMBitcode = false; break; } case FILE_TYPE_ISA: { @@ -1185,59 +1168,6 @@ bool Program::linkImplLC(amd::option::Options* options) { return false; } - // call LinkLLVMBitcode - if (bLinkLLVMBitcode) { - // open the bitcode libraries - std::vector linkOptions; - - if (options->oVariables->FP32RoundDivideSqrt) { - linkOptions.push_back("correctly_rounded_sqrt"); - } - if (options->oVariables->DenormsAreZero || AMD_GPU_FORCE_SINGLE_FP_DENORM == 0 || - (device().isa().versionMajor() < 9 && AMD_GPU_FORCE_SINGLE_FP_DENORM < 0)) { - linkOptions.push_back("daz_opt"); - } - if (options->oVariables->FiniteMathOnly || options->oVariables->FastRelaxedMath) { - linkOptions.push_back("finite_only"); - } - if (options->oVariables->UnsafeMathOpt || options->oVariables->FastRelaxedMath) { - linkOptions.push_back("unsafe_math"); - } - if (device().settings().lcWavefrontSize64_) { - linkOptions.push_back("wavefrontsize64"); - } - linkOptions.push_back("code_object_v" + std::to_string(options->oVariables->LCCodeObjectVersion)); - - amd_comgr_status_t status = addCodeObjData(llvmBinary_.data(), llvmBinary_.size(), - AMD_COMGR_DATA_KIND_BC, - "LLVM Binary", &inputs); - - amd_comgr_data_set_t linked_bc; - bool hasLinkedBC = false; - - if (status == AMD_COMGR_STATUS_SUCCESS) { - status = amd::Comgr::create_data_set(&linked_bc); - } - - bool ret = (status == AMD_COMGR_STATUS_SUCCESS); - if (ret) { - hasLinkedBC = true; - ret = linkLLVMBitcode(inputs, linkOptions, options, &linked_bc); - } - - amd::Comgr::destroy_data_set(inputs); - - if (!ret) { - if (hasLinkedBC) { - amd::Comgr::destroy_data_set(linked_bc); - } - buildLog_ += "Error: Linking bitcode failed: linking source & IR libraries.\n"; - return false; - } - - inputs = linked_bc; - } - std::vector codegenOptions; // TODO: Can this be fixed at the source? options->llvmOptions is a flat diff --git a/rocclr/device/devprogram.hpp b/rocclr/device/devprogram.hpp index 55eac9984e..13d90e27d2 100644 --- a/rocclr/device/devprogram.hpp +++ b/rocclr/device/devprogram.hpp @@ -457,8 +457,7 @@ 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, - const bool link_dev_libs = true); + char* binaryData[] = nullptr, size_t* binarySize = nullptr); //! Create the bitcode of the compiled input dataset bool compileToLLVMBitcode(const amd_comgr_data_set_t compileInputs,