From 44e1f1ba4b9df5bb2c0c48f22f83655e7bfb7ac3 Mon Sep 17 00:00:00 2001 From: Patryk Wrobel Date: Fri, 12 Aug 2022 16:57:46 +0000 Subject: [PATCH] Remove redundant copying of std::vectors Usage of initializer list in for loop to iterate over heavy types has bad consequences. std::initialize_list is only a view and its data is silently created as T[N]. Therefore, if someone uses std::vector with it, it will cause deep-copying of the elements. This change introduces usage of pointers on std::initializer_list to perform a shallow-copy of an addresses. Furthermore, it adds const references in few places, where copy is not needed. Signed-off-by: Patryk Wrobel --- .../offline_compiler/ocloc_fatbinary_tests.cpp | 15 ++++++++------- .../xe_hpc_core/pvc/test_hw_info_config_pvc.cpp | 8 ++++---- .../offline_compiler/source/ocloc_arg_helper.cpp | 2 +- shared/offline_compiler/source/ocloc_arg_helper.h | 12 ++++++------ .../source/device_binary_format/debug_zebin.cpp | 4 ++-- .../xe_hpg_core/dg2/hw_info_config_tests_dg2.cpp | 4 ++-- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/opencl/test/unit_test/offline_compiler/ocloc_fatbinary_tests.cpp b/opencl/test/unit_test/offline_compiler/ocloc_fatbinary_tests.cpp index 49cf334da8..a09994a635 100644 --- a/opencl/test/unit_test/offline_compiler/ocloc_fatbinary_tests.cpp +++ b/opencl/test/unit_test/offline_compiler/ocloc_fatbinary_tests.cpp @@ -261,8 +261,8 @@ TEST_F(OclocFatBinaryProductAcronymsTests, givenDeviceArgProvidedWhenUnknownFami } TEST_F(OclocFatBinaryProductAcronymsTests, givenDeviceArgProvidedWhenKnownNameIsPassedWithCoreSuffixThenRequestedFatBinaryReturnsTrue) { - for (const auto &acronyms : {enabledFamiliesAcronyms, enabledReleasesAcronyms}) { - for (const auto &acronym : acronyms) { + for (const auto *acronyms : {&enabledFamiliesAcronyms, &enabledReleasesAcronyms}) { + for (const auto &acronym : *acronyms) { auto acronymStr = acronym.str() + "_core"; const char *name[] = {"ocloc", "-device", acronymStr.c_str()}; EXPECT_TRUE(NEO::requestedFatBinary(3, name, oclocArgHelperWithoutInput.get())); @@ -271,8 +271,8 @@ TEST_F(OclocFatBinaryProductAcronymsTests, givenDeviceArgProvidedWhenKnownNameIs } TEST_F(OclocFatBinaryProductAcronymsTests, givenDeviceArgProvidedWhenKnownNameIsPassedThenRequestedFatBinaryReturnsTrue) { - for (const auto &acronyms : {enabledFamiliesAcronyms, enabledReleasesAcronyms}) { - for (const auto &acronym : acronyms) { + for (const auto *acronyms : {&enabledFamiliesAcronyms, &enabledReleasesAcronyms}) { + for (const auto &acronym : *acronyms) { auto acronymStr = acronym.str(); const char *name[] = {"ocloc", "-device", acronymStr.c_str()}; EXPECT_TRUE(NEO::requestedFatBinary(3, name, oclocArgHelperWithoutInput.get())); @@ -286,7 +286,8 @@ TEST_F(OclocFatBinaryProductAcronymsTests, givenUnkownArchitectureThenReturnEmpt } TEST_F(OclocFatBinaryProductAcronymsTests, givenClosedRangeTooExtensiveWhenProductsOrFamiliesOrReleasesAreValidThenFailIsReturned) { - for (const auto &enabledAcronyms : {enabledProductsAcronyms, enabledReleasesAcronyms, enabledFamiliesAcronyms}) { + for (const auto *enabledAcronymsPtr : {&enabledProductsAcronyms, &enabledReleasesAcronyms, &enabledFamiliesAcronyms}) { + const auto &enabledAcronyms = *enabledAcronymsPtr; if (enabledAcronyms.size() < 3) { GTEST_SKIP(); } @@ -323,8 +324,8 @@ TEST_F(OclocFatBinaryProductAcronymsTests, givenAcronymOpenRangeWhenAcronymIsUnk } TEST_F(OclocFatBinaryProductAcronymsTests, givenClosedRangeWhenAnyOfAcronymIsUnknownThenReturnEmptyList) { - for (const auto &vec : {enabledProductsAcronyms, enabledReleasesAcronyms, enabledFamiliesAcronyms}) { - for (const auto &acronym : vec) { + for (const auto *vec : {&enabledProductsAcronyms, &enabledReleasesAcronyms, &enabledFamiliesAcronyms}) { + for (const auto &acronym : *vec) { auto got = NEO::getTargetProductsForFatbinary("unk:" + acronym.str(), oclocArgHelperWithoutInput.get()); EXPECT_TRUE(got.empty()); diff --git a/opencl/test/unit_test/xe_hpc_core/pvc/test_hw_info_config_pvc.cpp b/opencl/test/unit_test/xe_hpc_core/pvc/test_hw_info_config_pvc.cpp index ded884d3bd..c333c96ccd 100644 --- a/opencl/test/unit_test/xe_hpc_core/pvc/test_hw_info_config_pvc.cpp +++ b/opencl/test/unit_test/xe_hpc_core/pvc/test_hw_info_config_pvc.cpp @@ -123,8 +123,8 @@ PVCTEST_F(PvcHwInfo, givenVariousValuesWhenConvertingHwRevIdAndSteppingThenConve const auto &hwInfoConfig = *HwInfoConfig::get(hwInfo.platform.eProductFamily); for (uint32_t testValue = 0; testValue < 0xFF; testValue++) { - for (const auto &pvc : {pvcXlDeviceIds, pvcXtDeviceIds}) { - for (const auto &deviceId : pvc) { + for (const auto *pvc : {&pvcXlDeviceIds, &pvcXtDeviceIds}) { + for (const auto &deviceId : *pvc) { hwInfo.platform.usDeviceID = deviceId; auto hwRevIdFromStepping = hwInfoConfig.getHwRevIdFromStepping(testValue, hwInfo); if (hwRevIdFromStepping != CommonConstants::invalidStepping) { @@ -137,8 +137,8 @@ PVCTEST_F(PvcHwInfo, givenVariousValuesWhenConvertingHwRevIdAndSteppingThenConve auto steppingFromHwRevId = hwInfoConfig.getSteppingFromHwRevId(hwInfo); if (steppingFromHwRevId != CommonConstants::invalidStepping) { bool anyMatchAfterConversionFromStepping = false; - for (const auto &pvc : {pvcXlDeviceIds, pvcXtDeviceIds}) { - for (const auto &deviceId : pvc) { + for (const auto *pvc : {&pvcXlDeviceIds, &pvcXtDeviceIds}) { + for (const auto &deviceId : *pvc) { hwInfo.platform.usDeviceID = deviceId; auto hwRevId = hwInfoConfig.getHwRevIdFromStepping(steppingFromHwRevId, hwInfo); EXPECT_NE(CommonConstants::invalidStepping, hwRevId); diff --git a/shared/offline_compiler/source/ocloc_arg_helper.cpp b/shared/offline_compiler/source/ocloc_arg_helper.cpp index 860c506217..6086956ed2 100644 --- a/shared/offline_compiler/source/ocloc_arg_helper.cpp +++ b/shared/offline_compiler/source/ocloc_arg_helper.cpp @@ -161,7 +161,7 @@ bool OclocArgHelper::getHwInfoForProductConfig(uint32_t productConfig, NEO::Hard return retVal; } - auto deviceAotMap = productConfigHelper->getDeviceAotInfo(); + const auto &deviceAotMap = productConfigHelper->getDeviceAotInfo(); for (auto &deviceConfig : deviceAotMap) { if (deviceConfig.aotConfig.ProductConfig == productConfig) { hwInfo = *deviceConfig.hwInfo; diff --git a/shared/offline_compiler/source/ocloc_arg_helper.h b/shared/offline_compiler/source/ocloc_arg_helper.h index 72ba4c4b63..d591bdafbe 100644 --- a/shared/offline_compiler/source/ocloc_arg_helper.h +++ b/shared/offline_compiler/source/ocloc_arg_helper.h @@ -110,10 +110,10 @@ class OclocArgHelper { } template - static auto getArgsWithoutDuplicate(Args... args) { + static auto getArgsWithoutDuplicate(const Args &...args) { std::vector out{}; - for (const auto &acronyms : {args...}) { - for (const auto &acronym : acronyms) { + for (const auto *acronyms : {std::addressof(args)...}) { + for (const auto &acronym : *acronyms) { if (!std::any_of(out.begin(), out.end(), findDuplicate(acronym))) { out.push_back(acronym); } @@ -123,10 +123,10 @@ class OclocArgHelper { } template - static auto createStringForArgs(Args... args) { + static auto createStringForArgs(const Args &...args) { std::ostringstream os; - for (const auto &acronyms : {args...}) { - for (const auto &acronym : acronyms) { + for (const auto *acronyms : {std::addressof(args)...}) { + for (const auto &acronym : *acronyms) { if (os.tellp()) os << ", "; os << acronym.str(); diff --git a/shared/source/device_binary_format/debug_zebin.cpp b/shared/source/device_binary_format/debug_zebin.cpp index ea39705cec..9947de4e2e 100644 --- a/shared/source/device_binary_format/debug_zebin.cpp +++ b/shared/source/device_binary_format/debug_zebin.cpp @@ -118,8 +118,8 @@ void DebugZebinCreator::applyRelocations() { } } - for (const auto &relocations : {elf.getDebugInfoRelocations(), elf.getRelocations()}) { - for (const auto &reloc : relocations) { + for (const auto *relocations : {&elf.getDebugInfoRelocations(), &elf.getRelocations()}) { + for (const auto &reloc : *relocations) { auto relocType = static_cast(reloc.relocType); if (isRelocTypeSupported(relocType) == false) { continue; diff --git a/shared/test/unit_test/xe_hpg_core/dg2/hw_info_config_tests_dg2.cpp b/shared/test/unit_test/xe_hpg_core/dg2/hw_info_config_tests_dg2.cpp index 8eb151c2c6..284a12c11a 100644 --- a/shared/test/unit_test/xe_hpg_core/dg2/hw_info_config_tests_dg2.cpp +++ b/shared/test/unit_test/xe_hpg_core/dg2/hw_info_config_tests_dg2.cpp @@ -451,8 +451,8 @@ DG2TEST_F(ProductConfigTests, givenDg2G10DeviceIdWhenDifferentRevisionIsPassedTh } DG2TEST_F(ProductConfigTests, givenDg2DeviceIdWhenIncorrectRevisionIsPassedThenCorrectProductConfigIsReturned) { - for (const auto &dg2 : {dg2G10DeviceIds, dg2G11DeviceIds}) { - for (const auto &deviceId : dg2) { + for (const auto *dg2 : {&dg2G10DeviceIds, &dg2G11DeviceIds}) { + for (const auto &deviceId : *dg2) { hwInfo.platform.usDeviceID = deviceId; hwInfo.platform.usRevId = CommonConstants::invalidRevisionID; productConfig = hwInfoConfig->getProductConfigFromHwInfo(hwInfo);