From fdf3ebad7627ddc6b16b7c35ec9e17d4bb7fb92f Mon Sep 17 00:00:00 2001 From: Jason Tang Date: Wed, 15 Jul 2020 11:17:11 -0400 Subject: [PATCH] SWDEV-232197 - Use TargetID for action_info_set_isa_name() Change-Id: I6661a2bbc2e55586c1b5029694d67cb54a4f23a6 [ROCm/clr commit: 65f42304943b57345d75be417b7131be89003001] --- projects/clr/rocclr/device/devkernel.cpp | 9 ++-- projects/clr/rocclr/device/devkernel.hpp | 6 +-- projects/clr/rocclr/device/devprogram.cpp | 46 ++++++------------- projects/clr/rocclr/device/devprogram.hpp | 17 ++----- projects/clr/rocclr/device/pal/paldevice.cpp | 2 + projects/clr/rocclr/device/pal/palprogram.cpp | 4 -- projects/clr/rocclr/device/pal/palprogram.hpp | 2 - projects/clr/rocclr/device/rocm/rockernel.cpp | 13 +----- .../clr/rocclr/device/rocm/rocprogram.cpp | 4 -- 9 files changed, 30 insertions(+), 73 deletions(-) diff --git a/projects/clr/rocclr/device/devkernel.cpp b/projects/clr/rocclr/device/devkernel.cpp index 0ad9b4b2c4..f12431da3b 100755 --- a/projects/clr/rocclr/device/devkernel.cpp +++ b/projects/clr/rocclr/device/devkernel.cpp @@ -1155,7 +1155,7 @@ bool Kernel::GetAttrCodePropMetadata() { return true; } -bool Kernel::SetAvailableSgprVgpr(const std::string& targetIdent) { +bool Kernel::SetAvailableSgprVgpr() { std::string buf; amd_comgr_metadata_node_t isaMeta; @@ -1165,7 +1165,8 @@ bool Kernel::SetAvailableSgprVgpr(const std::string& targetIdent) { bool hasSgprMeta = false; bool hasVgprMeta = false; - amd_comgr_status_t status = amd::Comgr::get_isa_metadata(targetIdent.c_str(), &isaMeta); + amd_comgr_status_t status = amd::Comgr::get_isa_metadata( + prog().device().info().targetId_, &isaMeta); if (status == AMD_COMGR_STATUS_SUCCESS) { hasIsaMeta = true; @@ -1300,7 +1301,7 @@ void Kernel::InitParameters(const amd_comgr_metadata_node_t kernelMD) { return; } - // COMGR has unclear/undefined order of the fields filling. + // COMGR has unclear/undefined order of the fields filling. // Correct the types for the abstraciton layer after all fields are available if (desc.info_.oclObject_ != amd::KernelParameterDescriptor::ValueObject) { switch (desc.info_.oclObject_) { @@ -1350,7 +1351,7 @@ void Kernel::InitParameters(const amd_comgr_metadata_node_t kernelMD) { hiddenParams.push_back(desc); continue; } - + // These objects have forced data size to uint64_t if (codeObjectVer() == 2) { if ((desc.info_.oclObject_ == amd::KernelParameterDescriptor::ImageObject) || diff --git a/projects/clr/rocclr/device/devkernel.hpp b/projects/clr/rocclr/device/devkernel.hpp index 82cc9e9120..d164f76a76 100644 --- a/projects/clr/rocclr/device/devkernel.hpp +++ b/projects/clr/rocclr/device/devkernel.hpp @@ -61,7 +61,7 @@ struct KernelParameterDescriptor { uint32_t defined_ : 1; //!< The argument was defined by the app uint32_t hidden_ : 1; //!< It's a hidden argument uint32_t shared_ : 1; //!< Dynamic shared memory - uint32_t reserved_ : 3; //!< Reserved + uint32_t reserved_ : 3; //!< Reserved uint32_t arrayIndex_ : 20; //!< Index in the objects array or LDS alignment }; uint32_t allValues_; @@ -463,7 +463,7 @@ class Kernel : public amd::HeapObject { const uint32_t WorkitemPrivateSegmentByteSize() const { return workitemPrivateSegmentByteSize_; } void SetWorkitemPrivateSegmentByteSize(uint32_t size) { workitemPrivateSegmentByteSize_ = size; } - + const uint32_t KernargSegmentByteSize() const { return kernargSegmentByteSize_; } void SetKernargSegmentByteSize(uint32_t size) { kernargSegmentByteSize_ = size; } @@ -481,7 +481,7 @@ class Kernel : public amd::HeapObject { bool GetAttrCodePropMetadata(); //! Retrieve the available SGPRs and VGPRs - bool SetAvailableSgprVgpr(const std::string& targetIdent); + bool SetAvailableSgprVgpr(); //! Retrieve the printf string metadata bool GetPrintfStr(std::vector* printfStr); diff --git a/projects/clr/rocclr/device/devprogram.cpp b/projects/clr/rocclr/device/devprogram.cpp index 64387b3dd9..e81857d926 100755 --- a/projects/clr/rocclr/device/devprogram.cpp +++ b/projects/clr/rocclr/device/devprogram.cpp @@ -246,8 +246,7 @@ amd_comgr_status_t Program::addCodeObjData(const char *source, return status; } -void Program::setLangAndTargetStr(const char* clStd, amd_comgr_language_t* langver, - std::string& targetIdent) { +void Program::setLanguage(const char* clStd, amd_comgr_language_t* langver) { if (isHIP()) { if (langver != nullptr) { @@ -272,20 +271,10 @@ void Program::setLangAndTargetStr(const char* clStd, amd_comgr_language_t* langv } } } - // Set target triple and CPU - targetIdent = std::string("amdgcn-amd-amdhsa--") + machineTarget_; - // Set xnack option if needed - if (xnackEnabled_) { - targetIdent.append("+xnack"); - } - if (sramEccEnabled_) { - targetIdent.append("+sram-ecc"); - } } amd_comgr_status_t Program::createAction(const amd_comgr_language_t oclver, - const std::string& targetIdent, const std::vector& options, amd_comgr_action_info_t* action, bool* hasAction) { @@ -300,8 +289,8 @@ amd_comgr_status_t Program::createAction(const amd_comgr_language_t oclver, } } - if (!targetIdent.empty() && (status == AMD_COMGR_STATUS_SUCCESS)) { - status = amd::Comgr::action_info_set_isa_name(*action, targetIdent.c_str()); + if (status == AMD_COMGR_STATUS_SUCCESS) { + status = amd::Comgr::action_info_set_isa_name(*action, device().info().targetId_); } if (status == AMD_COMGR_STATUS_SUCCESS) { @@ -325,10 +314,8 @@ bool Program::linkLLVMBitcode(const amd_comgr_data_set_t inputs, amd::option::Options* amdOptions, amd_comgr_data_set_t* output, char* binaryData[], size_t* binarySize) { - // get the language and target name - std::string targetIdent; amd_comgr_language_t langver; - setLangAndTargetStr(amdOptions->oVariables->CLStd, &langver, targetIdent); + setLanguage(amdOptions->oVariables->CLStd, &langver); if (langver == AMD_COMGR_LANGUAGE_NONE) { DevLogPrintfError("Cannot set Langauge version for %s \n", amdOptions->oVariables->CLStd); @@ -341,7 +328,7 @@ bool Program::linkLLVMBitcode(const amd_comgr_data_set_t inputs, bool hasAction = false; bool hasDataSetDevLibs = false; - amd_comgr_status_t status = createAction(langver, targetIdent, options, &action, &hasAction); + amd_comgr_status_t status = createAction(langver, options, &action, &hasAction); if (status == AMD_COMGR_STATUS_SUCCESS) { status = amd::Comgr::create_data_set(&dataSetDevLibs); @@ -381,13 +368,12 @@ bool Program::linkLLVMBitcode(const amd_comgr_data_set_t inputs, } bool Program::compileToLLVMBitcode(const amd_comgr_data_set_t compileInputs, - const std::vector& options, amd::option::Options* amdOptions, + const std::vector& options, + amd::option::Options* amdOptions, char* binaryData[], size_t* binarySize) { - // get the lanuage and target name - std::string targetIdent; amd_comgr_language_t langver; - setLangAndTargetStr(amdOptions->oVariables->CLStd, &langver, targetIdent); + setLanguage(amdOptions->oVariables->CLStd, &langver); if (langver == AMD_COMGR_LANGUAGE_NONE) { DevLogPrintfError("Cannot set Langauge version for %s \n", amdOptions->oVariables->CLStd); @@ -404,7 +390,7 @@ bool Program::compileToLLVMBitcode(const amd_comgr_data_set_t compileInputs, bool hasOutput = false; bool hasDataSetPCH = false; - amd_comgr_status_t status = createAction(langver, targetIdent, options, &action, &hasAction); + amd_comgr_status_t status = createAction(langver, options, &action, &hasAction); if (status == AMD_COMGR_STATUS_SUCCESS) { status = amd::Comgr::create_data_set(&output); @@ -494,13 +480,10 @@ bool Program::compileToLLVMBitcode(const amd_comgr_data_set_t compileInputs, // the input data set is converted to relocatable code, then executable binary. // If assembly code is required, the input data set is converted to assembly. bool Program::compileAndLinkExecutable(const amd_comgr_data_set_t inputs, - const std::vector& options, amd::option::Options* amdOptions, + const std::vector& options, + amd::option::Options* amdOptions, char* executable[], size_t* executableSize) { - // get the language and target name - std::string targetIdent; - setLangAndTargetStr(amdOptions->oVariables->CLStd, nullptr, targetIdent); - // create the linked output amd_comgr_action_info_t action; amd_comgr_data_set_t output; @@ -509,8 +492,7 @@ bool Program::compileAndLinkExecutable(const amd_comgr_data_set_t inputs, bool hasOutput = false; bool hasRelocatableData = false; - amd_comgr_status_t status = createAction(AMD_COMGR_LANGUAGE_NONE, targetIdent, options, - &action, &hasAction); + amd_comgr_status_t status = createAction(AMD_COMGR_LANGUAGE_NONE, options, &action, &hasAction); if (status == AMD_COMGR_STATUS_SUCCESS) { status = amd::Comgr::create_data_set(&output); @@ -1188,8 +1170,8 @@ bool Program::linkImplLC(amd::option::Options* options) { codegenOptions.insert(codegenOptions.end(), options->clangOptions.begin(), options->clangOptions.end()); - // Set SRAM ECC option if needed - if (sramEccEnabled_) { + // TODO: SRAM ECC option will be removed when Target ID feature is fully implemented + if (device().info().sramEccEnabled_) { codegenOptions.push_back("-msram-ecc"); } else { diff --git a/projects/clr/rocclr/device/devprogram.hpp b/projects/clr/rocclr/device/devprogram.hpp index 85731857a8..dfb0a8ce2d 100755 --- a/projects/clr/rocclr/device/devprogram.hpp +++ b/projects/clr/rocclr/device/devprogram.hpp @@ -96,8 +96,6 @@ class Program : public amd::HeapObject { uint32_t internal_ : 1; //!< Internal blit program uint32_t isLC_ : 1; //!< LC was used for the program compilation uint32_t hasGlobalStores_ : 1; //!< Program has writable program scope variables - uint32_t xnackEnabled_ : 1; //!< Xnack was enabled during compilation - uint32_t sramEccEnabled_ : 1; //!< SRAM ECC was enabled during compilation uint32_t isHIP_ : 1; //!< Determine if the program is for HIP }; uint32_t flags_; //!< Program flags @@ -237,12 +235,6 @@ class Program : public amd::HeapObject { //! Get the machine target for the program const char* machineTarget() const { return machineTarget_; } - //! Check if xnack is enable - const bool xnackEnable() const { return (xnackEnabled_ == 1); } - - //! Check if SRAM ECC is enable - const bool sramEccEnable() const { return (sramEccEnabled_ == 1); } - //! Check if program is HIP based const bool isHIP() const { return (isHIP_ == 1); } @@ -378,9 +370,8 @@ class Program : public amd::HeapObject { const amd_comgr_data_kind_t dataKind, const std::string& outFileName, char* outBinary[] = nullptr, size_t* outSize = nullptr); - //! Set the OCL language and target triples with feature - void setLangAndTargetStr(const char* clStd, amd_comgr_language_t* oclver, - std::string& targetIdent); + //! Set the OCL language + void setLanguage(const char* clStd, amd_comgr_language_t* oclver); //! Create code object and add it into the data set amd_comgr_status_t addCodeObjData(const char *source, @@ -389,8 +380,8 @@ class Program : public amd::HeapObject { //! Create action for the specified language, target and options amd_comgr_status_t createAction(const amd_comgr_language_t oclvar, - const std::string& targetIdent, const std::vector& options, - amd_comgr_action_info_t* action, bool* hasAction); + const std::vector& options, amd_comgr_action_info_t* action, + bool* hasAction); //! Create the bitcode of the linked input dataset bool linkLLVMBitcode(const amd_comgr_data_set_t inputs, diff --git a/projects/clr/rocclr/device/pal/paldevice.cpp b/projects/clr/rocclr/device/pal/paldevice.cpp index 44e7a4c679..bb82b4687e 100755 --- a/projects/clr/rocclr/device/pal/paldevice.cpp +++ b/projects/clr/rocclr/device/pal/paldevice.cpp @@ -576,6 +576,8 @@ void NullDevice::fillDeviceInfo(const Pal::DeviceProperties& palProp, if (info_.sramEccEnabled_) { ::strcat(info_.name_, "+sram-ecc"); } + ::strcpy(info_.targetId_, "amdgcn-amd-amdhsa--"); + ::strcat(info_.targetId_, info_.name_); } else { ::strcpy(info_.name_, hwInfo()->machineTarget_); } diff --git a/projects/clr/rocclr/device/pal/palprogram.cpp b/projects/clr/rocclr/device/pal/palprogram.cpp index d18f1c4f30..83a4fe3c4c 100644 --- a/projects/clr/rocclr/device/pal/palprogram.cpp +++ b/projects/clr/rocclr/device/pal/palprogram.cpp @@ -180,8 +180,6 @@ HSAILProgram::HSAILProgram(Device& device, amd::Program& owner) maxScratchRegs_(0), executable_(nullptr), loaderContext_(this) { - xnackEnabled_ = dev().hwInfo()->xnackEnabled_; - sramEccEnabled_ = dev().info().sramEccEnabled_; if (dev().asicRevision() == Pal::AsicRevision::Bristol) { machineTarget_ = Carrizo; } else { @@ -200,8 +198,6 @@ HSAILProgram::HSAILProgram(NullDevice& device, amd::Program& owner) executable_(nullptr), loaderContext_(this) { isNull_ = true; - xnackEnabled_ = dev().hwInfo()->xnackEnabled_; - sramEccEnabled_ = dev().info().sramEccEnabled_; if (dev().asicRevision() == Pal::AsicRevision::Bristol) { machineTarget_ = Carrizo; } else { diff --git a/projects/clr/rocclr/device/pal/palprogram.hpp b/projects/clr/rocclr/device/pal/palprogram.hpp index 7abdbb2cef..86dc6da657 100644 --- a/projects/clr/rocclr/device/pal/palprogram.hpp +++ b/projects/clr/rocclr/device/pal/palprogram.hpp @@ -246,8 +246,6 @@ class LightningProgram : public HSAILProgram { LightningProgram(Device& device, amd::Program& owner) : HSAILProgram(device, owner) { isLC_ = true; isHIP_ = (owner.language() == amd::Program::HIP); - xnackEnabled_ = dev().hwInfo()->xnackEnabled_; - machineTarget_ = dev().hwInfo()->machineTargetLC_; } virtual ~LightningProgram() {} diff --git a/projects/clr/rocclr/device/rocm/rockernel.cpp b/projects/clr/rocclr/device/rocm/rockernel.cpp index e7ff95adf1..5c5eaca48f 100644 --- a/projects/clr/rocclr/device/rocm/rockernel.cpp +++ b/projects/clr/rocclr/device/rocm/rockernel.cpp @@ -63,17 +63,8 @@ bool LightningKernel::init() { workGroupInfo_.availableLDSSize_ = dev().info().localMemSizePerCU_; assert(workGroupInfo_.availableLDSSize_ > 0); - // Get the available SGPRs and VGPRs - std::string targetIdent = std::string("amdgcn-amd-amdhsa--")+program()->machineTarget(); - if (program()->xnackEnable()) { - targetIdent.append("+xnack"); - } - if (program()->sramEccEnable()) { - targetIdent.append("+sram-ecc"); - } - - if (!SetAvailableSgprVgpr(targetIdent)) { - DevLogPrintfError("Cannot set available SGPR/VGPR for target Ident:%s \n", targetIdent.c_str()); + if (!SetAvailableSgprVgpr()) { + DevLogError("Cannot set available SGPR/VGPR\n"); return false; } diff --git a/projects/clr/rocclr/device/rocm/rocprogram.cpp b/projects/clr/rocclr/device/rocm/rocprogram.cpp index 977ce3442f..98ae60c55b 100644 --- a/projects/clr/rocclr/device/rocm/rocprogram.cpp +++ b/projects/clr/rocclr/device/rocm/rocprogram.cpp @@ -230,8 +230,6 @@ bool Program::createGlobalVarObj(amd::Memory** amd_mem_obj, void** device_pptr, } HSAILProgram::HSAILProgram(roc::NullDevice& device, amd::Program& owner) : roc::Program(device, owner) { - xnackEnabled_ = dev().settings().enableXNACK_; - sramEccEnabled_ = dev().info().sramEccEnabled_; machineTarget_ = dev().deviceInfo().complibTarget_; } @@ -437,8 +435,6 @@ LightningProgram::LightningProgram(roc::NullDevice& device, amd::Program& owner) : roc::Program(device, owner) { isLC_ = true; isHIP_ = (owner.language() == amd::Program::HIP); - xnackEnabled_ = dev().settings().enableXNACK_; - sramEccEnabled_ = dev().info().sramEccEnabled_; machineTarget_ = dev().deviceInfo().machineTargetLC_; }