From 676a364fb384c1c12fd26b2dd45135ad6f9cfb80 Mon Sep 17 00:00:00 2001 From: "Sabianin, Maksim" Date: Thu, 1 Feb 2024 08:57:22 -0800 Subject: [PATCH 1/3] [SYCL] Make PropertySetRegistry owning it's content Before this patch PropertySetRegistry used StringRefs that point at external data that was supposed to outlive the instance of PropertySetRegistry. It wasn't obvious and it might lead to memory bugs. This patch makes PropertySetRegistry owning it's content as containers usually do. Also the previous implementation had a property that a content is being printed in the order it was added. This patch removes this property. However, the patch brings a sorted order in write() method in order to make testing as easy as it was. Accordingly, all affected tests were aligned to the sorted order. Also documentation is aligned with doxygen BKSM. --- .../SYCLOffloadWrapper.cpp | 4 +- .../ClangOffloadWrapper.cpp | 4 +- llvm/include/llvm/Support/PropertySetIO.h | 50 +++++++++++-------- llvm/lib/Support/PropertySetIO.cpp | 22 ++++++-- ...CL-2020-zeroinitializer-array-of-arrays.ll | 2 +- .../spec-constants/SYCL-2020.ll | 19 +++---- .../SYCL2020-struct-with-undef-padding.ll | 1 + .../composite-default-value-padding.ll | 2 +- llvm/tools/sycl-post-link/sycl-post-link.cpp | 19 ++++--- 9 files changed, 75 insertions(+), 48 deletions(-) diff --git a/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp b/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp index 9d8dfca5cee8f..7f6bcb6310fb1 100644 --- a/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp +++ b/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp @@ -418,7 +418,7 @@ struct Wrapper { addPropertySetToModule(const PropertySet &PropSet) { SmallVector PropInits; for (const auto &Prop : PropSet) { - Constant *PropName = addStringToModule(Prop.first, "prop"); + Constant *PropName = addStringToModule(Prop.first(), "prop"); Constant *PropValAddr = nullptr; Constant *PropType = ConstantInt::get(Type::getInt32Ty(C), Prop.second.getType()); @@ -497,7 +497,7 @@ struct Wrapper { std::pair Props = addPropertySetToModule(PropSet.second); // get the next the middle column element - auto *Category = addStringToModule(PropSet.first, "SYCL_PropSetName"); + auto *Category = addStringToModule(PropSet.first(), "SYCL_PropSetName"); PropSetsInits.push_back(ConstantStruct::get(SyclPropSetTy, Category, Props.first, Props.second)); } diff --git a/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp b/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp index 5facbb4329abd..4a52660610ed6 100644 --- a/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp +++ b/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp @@ -817,7 +817,7 @@ class BinaryWrapper { std::vector PropInits; for (const auto &Prop : PropSet) { - Constant *PropName = addStringToModule(Prop.first, "prop"); + Constant *PropName = addStringToModule(Prop.first(), "prop"); Constant *PropValAddr = nullptr; Constant *PropType = ConstantInt::get(Type::getInt32Ty(C), Prop.second.getType()); @@ -921,7 +921,7 @@ class BinaryWrapper { if (!Props) return Props.takeError(); // get the next the middle column element - auto *Category = addStringToModule(PropSet.first, "SYCL_PropSetName"); + auto *Category = addStringToModule(PropSet.first(), "SYCL_PropSetName"); PropSetsInits.push_back(ConstantStruct::get( getSyclPropSetTy(), Category, Props.get().first, Props.get().second)); } diff --git a/llvm/include/llvm/Support/PropertySetIO.h b/llvm/include/llvm/Support/PropertySetIO.h index cb257ba7e5c77..9bbffa356be58 100644 --- a/llvm/include/llvm/Support/PropertySetIO.h +++ b/llvm/include/llvm/Support/PropertySetIO.h @@ -33,7 +33,7 @@ #ifndef LLVM_SUPPORT_PROPERTYSETIO_H #define LLVM_SUPPORT_PROPERTYSETIO_H -#include "llvm/ADT/MapVector.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" @@ -175,14 +175,18 @@ class PropertyValue { } Val; }; -// A property set. Preserves insertion order when iterating elements. -using PropertySet = MapVector; +/// A property set. Doesn't preserves order of elements. +using PropertySet = StringMap; -// A "registry" of multiple property sets. Maps a property set name to its -// contents. Can be read/written. +/// A registry of property sets. Maps a property set name to its +/// content. +/// +/// The order of keys is not preserved because Hash Map is used. +/// However, write() method prints content sorted by Category names +/// and sorted by keys. This is made only for testing, don't rely on this. class PropertySetRegistry { public: - using MapTy = MapVector; + using MapTy = StringMap; // Specific property category names used by tools. static constexpr char SYCL_SPECIALIZATION_CONSTANTS[] = @@ -199,44 +203,50 @@ class PropertySetRegistry { static constexpr char SYCL_DEVICE_REQUIREMENTS[] = "SYCL/device requirements"; static constexpr char SYCL_HOST_PIPES[] = "SYCL/host pipes"; - // Function for bulk addition of an entire property set under given category - // (property set name). + /// Function for bulk addition of an entire property set in the given + /// \p Category . template void add(StringRef Category, const MapTy &Props) { - using KeyTy = typename MapTy::value_type::first_type; - static_assert(std::is_same::type, - llvm::StringRef>::value, - "wrong key type"); + assert(PropSetMap.find(Category) == PropSetMap.end() && + "category already added"); + auto &PropSet = PropSetMap[Category]; + + for (const auto &Prop : Props) + PropSet.insert_or_assign(Prop.first, PropertyValue(Prop.second)); + } + /// Function for bulk addition of an entire property set in the given \p + /// Category . + template + void add(StringRef Category, const StringMap &Props) { assert(PropSetMap.find(Category) == PropSetMap.end() && "category already added"); auto &PropSet = PropSetMap[Category]; for (const auto &Prop : Props) - PropSet.insert({Prop.first, PropertyValue(Prop.second)}); + PropSet.insert_or_assign(Prop.first(), PropertyValue(Prop.getValue())); } - // Function to add a property to a given category (property set name). + /// Adds the given \p PropVal with the given \p PropName into the given \p + /// Category . template void add(StringRef Category, StringRef PropName, const T &PropVal) { auto &PropSet = PropSetMap[Category]; PropSet.insert({PropName, PropertyValue(PropVal)}); } - // Parses and creates a property set registry. + /// Parses from the given \p Buf a property set registry. static Expected> read(const MemoryBuffer *Buf); - // Dumps a property set registry to a stream. + /// Dumps the property set registry to the given \p Out stream. void write(raw_ostream &Out) const; - // Start iterator of all preperty sets in the registry. MapTy::const_iterator begin() const { return PropSetMap.begin(); } - // End iterator of all preperty sets in the registry. MapTy::const_iterator end() const { return PropSetMap.end(); } - // Retrieves a property set with given name. + /// Retrieves a property set with given \p Name . PropertySet &operator[](StringRef Name) { return PropSetMap[Name]; } - // Constant access to the underlying map. + /// Constant access to the underlying map. const MapTy &getPropSets() const { return PropSetMap; } private: diff --git a/llvm/lib/Support/PropertySetIO.cpp b/llvm/lib/Support/PropertySetIO.cpp index 2a3586ba49339..d91e18f3df397 100644 --- a/llvm/lib/Support/PropertySetIO.cpp +++ b/llvm/lib/Support/PropertySetIO.cpp @@ -122,11 +122,23 @@ raw_ostream &operator<<(raw_ostream &Out, const PropertyValue &Prop) { } // namespace llvm void PropertySetRegistry::write(raw_ostream &Out) const { - for (const auto &PropSet : PropSetMap) { - Out << "[" << PropSet.first << "]\n"; - - for (const auto &Props : PropSet.second) { - Out << std::string(Props.first) << "=" << Props.second << "\n"; + SmallVector Keys1(PropSetMap.size()); + for (const auto &[I, PS] : enumerate(PropSetMap)) + Keys1[I] = PS.first(); + + sort(Keys1.begin(), Keys1.end()); + for (auto K1 : Keys1) { + const auto &PropSet = PropSetMap.at(K1); + Out << "[" << K1 << "]\n"; + + SmallVector Keys2(PropSet.size()); + for (const auto &[I, Props] : enumerate(PropSet)) + Keys2[I] = Props.first(); + + sort(Keys2.begin(), Keys2.end()); + for (auto K2 : Keys2) { + const auto &Prop = PropSet.at(K2); + Out << K2 << "=" << Prop << "\n"; } } } diff --git a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll index daa98b590d48a..27c395195763e 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll @@ -108,9 +108,9 @@ attributes #2 = { convergent "frame-pointer"="all" "no-trapping-math"="true" "st !9 = !{!"_ZTSN2cl4sycl14kernel_handlerE", !5, i64 0} ; CHECK-PROP: [SYCL/specialization constants] -; CHECK-PROP-NEXT: 9f47062a80eecfa7____ZL8coeff_id=2|gNAAAAAAAAAAAAAAAAAAAQAAAAQAAAAAEAAAAQAAAAgAAAAAIAAAAQAAAAwAAAAAMAAAAQAAAAABAAAAQAAAAQAAAAQBAAAAUAAAAQAAAAgBAAAAYAAAAQAAAAwBAAAAcAAAAQAAAAACAAAAgAAAAQAAAAA ; CHECK-PROP-NEXT: 405761736d5a1797____ZL9coeff_id2=2|gNAAAAAAAAQCAAAAAAAAAQAAAAgCAAAAEAAAAQAAAAwCAAAAIAAAAQAAAAADAAAAMAAAAQAAAAQDAAAAQAAAAQAAAAgDAAAAUAAAAQAAAAwDAAAAYAAAAQAAAAAEAAAAcAAAAQAAAAQEAAAAgAAAAQAAAAA ; CHECK-PROP-NEXT: 6da74a122db9f35d____ZL9coeff_id3=2|AGAAAAAAAAgEAAAAAAAAAQAAAAwEAAAAEAAAAQAAAAAFAAAAIAAAAQAAAAQFAAAAQAAAAgAAAAA +; CHECK-PROP-NEXT: 9f47062a80eecfa7____ZL8coeff_id=2|gNAAAAAAAAAAAAAAAAAAAQAAAAQAAAAAEAAAAQAAAAgAAAAAIAAAAQAAAAwAAAAAMAAAAQAAAAABAAAAQAAAAQAAAAQBAAAAUAAAAQAAAAgBAAAAYAAAAQAAAAwBAAAAcAAAAQAAAAACAAAAgAAAAQAAAAA ; CHECK-PROP: [SYCL/specialization constants default values] ; CHECK-PROP-NEXT: all=2|AMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAg/AAAAA0MzMIQzMzoAZmZGDEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA diff --git a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll index fb155fa86ffa0..826b8752c8b9b 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll @@ -266,14 +266,15 @@ attributes #3 = { nounwind } ; CHECK-RT-SAME: i32 [[#SCID17]], i32 4, i32 4} ; CHECK-PROPS: [SYCL/specialization constants] -; CHECK-PROPS: _ZTS14name_generatorIL_Z9id_halfEE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z6id_intEE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z9id_composEE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z10id_compos2EE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z10id_vectorEE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z10id_marrayEE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z10id_marray2EE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z10id_marray3EE=2| -; CHECK-PROPS: _ZTS14name_generatorIL_Z10id_marray4EE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_compos2EE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_compos3EE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marray2EE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marray3EE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marray4EE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marrayEE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_vectorEE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z6id_intEE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z9id_composEE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z9id_halfEE=2| ; CHECK-PROPS-DEF: [SYCL/specialization constants default values] ; CHECK-PROPS-DEF: all=2| diff --git a/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll b/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll index 8ed59930f737a..2b61600afc909 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll @@ -83,6 +83,7 @@ attributes #4 = { convergent } ; CHECK-IR: ![[#MN3]] = !{%struct.coeff2_str_aligned_t { %"class.std::array" zeroinitializer, i64 0, [7 x i8] undef, i8 undef }} ; CHECK-PROP: [SYCL/specialization constants] +; CHECK-PROP-NEXT: df991fa0adf9bad8____ZL8coeff_id2=2| ; CHECK-PROP-NEXT: ef880fa09cf7a9d7____ZL8coeff_id=2| ; CHECK-PROP: [SYCL/specialization constants default values] diff --git a/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll b/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll index a7e446e67dfab..e50490e73602a 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll @@ -96,8 +96,8 @@ attributes #5 = { nounwind } ; Most important information from the corresponding encoded data is the size of ; the specialization constants, i.e. 8 and 1 bytes respectively. ; CHECK: [SYCL/specialization constants] -; CHECK-NEXT: 9d329ad59055e972____ZL12StructSpecId=2|gBAAAAAAAAAAAAAAAAAAAgAAAAA ; CHECK-NEXT: 9d329ad59055e972____ZL10BoolSpecId=2|gBAAAAAAAAQAAAAAAAAAAEAAAAA +; CHECK-NEXT: 9d329ad59055e972____ZL12StructSpecId=2|gBAAAAAAAAAAAAAAAAAAAgAAAAA ; Ensure that the default values are correct. ; IBAAAAAAAAAFAAAAjBAAAEA is decoded to "0x48 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x14 diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 69f43b8b486ea..f0a4f59f56995 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -453,7 +453,8 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, if (KernelReqdWorkGroupSize.empty()) continue; MetadataNames.push_back(Func.getName().str() + "@reqd_work_group_size"); - ProgramMetadata.insert({MetadataNames.back(), KernelReqdWorkGroupSize}); + ProgramMetadata.insert_or_assign(MetadataNames.back(), + KernelReqdWorkGroupSize); } // Add global_id_mapping information with mapping between device-global @@ -464,11 +465,12 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, StringRef GlobalID = getGlobalVariableUniqueId(GV); MetadataNames.push_back(GlobalID.str() + "@global_id_mapping"); - ProgramMetadata.insert({MetadataNames.back(), GV.getName()}); + ProgramMetadata.insert_or_assign(MetadataNames.back(), GV.getName()); } } if (MD.isESIMD()) { - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert({"isEsimdImage", true}); + PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign("isEsimdImage", + true); } { StringRef RegAllocModeAttr = "sycl-register-alloc-mode"; @@ -534,17 +536,18 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, } if (OptLevel != -1) - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert({"optLevel", OptLevel}); + PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign("optLevel", + OptLevel); } { std::vector FuncNames = getKernelNamesUsingAssert(M); for (const StringRef &FName : FuncNames) - PropSet[PropSetRegTy::SYCL_ASSERT_USED].insert({FName, true}); + PropSet[PropSetRegTy::SYCL_ASSERT_USED].insert_or_assign(FName, true); } { if (isModuleUsingAsan(M)) - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert({"asanUsed", true}); + PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign("asanUsed", true); } if (GlobProps.EmitDeviceGlobalPropSet) { @@ -560,8 +563,8 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, } if (MD.isSpecConstantDefault()) - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert( - {"specConstsReplacedWithDefault", 1}); + PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign( + "specConstsReplacedWithDefault", 1); std::error_code EC; std::string SCFile = makeResultFileName(".prop", I, Suff); From 69b512454277149c3d266676812147a98cff7c9b Mon Sep 17 00:00:00 2001 From: "Sabianin, Maksim" Date: Mon, 5 Feb 2024 05:01:19 -0800 Subject: [PATCH 2/3] fix one test and remove unnecessary headers --- llvm/include/llvm/Support/PropertySetIO.h | 6 ------ sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp | 6 +++--- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/llvm/include/llvm/Support/PropertySetIO.h b/llvm/include/llvm/Support/PropertySetIO.h index 9bbffa356be58..dd0fb1c7f863b 100644 --- a/llvm/include/llvm/Support/PropertySetIO.h +++ b/llvm/include/llvm/Support/PropertySetIO.h @@ -39,12 +39,6 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/raw_ostream.h" -#include -#include -#include -#include -#include - namespace llvm { namespace util { diff --git a/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp b/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp index 762d73983db32..ca86ff3c213ed 100644 --- a/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp +++ b/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp @@ -46,11 +46,11 @@ int main() { } // CHECK-PROP: [SYCL/specialization constants] -// CHECK-PROP-NEXT: [[UNIQUE_PREFIX:[a-z0-9]+]]____ZL5Val23 -// CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL10ConstantId +// CHECK-PROP-NEXT: [[UNIQUE_PREFIX:[a-z0-9]+]]____ZL10ConstantId // CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL11SecondValue // CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL11SpecConst42 -// +// CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL5Val23 + // CHECK-IR: !sycl.specialization-constants = !{![[#MD0:]], ![[#MD1:]], ![[#MD2:]], ![[#MD3:]]} // CHECK-IR: ![[#MD0]] = !{!"[[UNIQUE_PREFIX:[a-z0-9]+]]____ZL5Val23", i32 [[#ID:]] // CHECK-IR: ![[#MD1]] = !{!"[[UNIQUE_PREFIX]]____ZL10ConstantId", i32 [[#ID+1]] From 22bc17fe7d13b7fec7d60334fd6af9685be77e08 Mon Sep 17 00:00:00 2001 From: "Sabianin, Maksim" Date: Fri, 9 Feb 2024 08:46:20 -0800 Subject: [PATCH 3/3] Address issues in PR Fix failing test in sycl/test/basic_tests. Get order of elements preserved as it was initially. Align tests correspondingly. --- .../SYCLOffloadWrapper.cpp | 4 +- .../ClangOffloadWrapper.cpp | 4 +- llvm/include/llvm/Support/PropertySetIO.h | 38 +++++++++---------- llvm/lib/Support/PropertySetIO.cpp | 22 +++-------- ...CL-2020-zeroinitializer-array-of-arrays.ll | 2 +- .../spec-constants/SYCL-2020.ll | 10 ++--- .../SYCL2020-struct-with-undef-padding.ll | 2 +- .../composite-default-value-padding.ll | 2 +- llvm/tools/sycl-post-link/sycl-post-link.cpp | 32 +++++++--------- .../SYCL-2020-spec-const-ids-order.cpp | 4 +- 10 files changed, 52 insertions(+), 68 deletions(-) diff --git a/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp b/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp index 7f6bcb6310fb1..9d8dfca5cee8f 100644 --- a/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp +++ b/clang/tools/clang-linker-wrapper/SYCLOffloadWrapper.cpp @@ -418,7 +418,7 @@ struct Wrapper { addPropertySetToModule(const PropertySet &PropSet) { SmallVector PropInits; for (const auto &Prop : PropSet) { - Constant *PropName = addStringToModule(Prop.first(), "prop"); + Constant *PropName = addStringToModule(Prop.first, "prop"); Constant *PropValAddr = nullptr; Constant *PropType = ConstantInt::get(Type::getInt32Ty(C), Prop.second.getType()); @@ -497,7 +497,7 @@ struct Wrapper { std::pair Props = addPropertySetToModule(PropSet.second); // get the next the middle column element - auto *Category = addStringToModule(PropSet.first(), "SYCL_PropSetName"); + auto *Category = addStringToModule(PropSet.first, "SYCL_PropSetName"); PropSetsInits.push_back(ConstantStruct::get(SyclPropSetTy, Category, Props.first, Props.second)); } diff --git a/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp b/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp index 4a52660610ed6..5facbb4329abd 100644 --- a/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp +++ b/clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp @@ -817,7 +817,7 @@ class BinaryWrapper { std::vector PropInits; for (const auto &Prop : PropSet) { - Constant *PropName = addStringToModule(Prop.first(), "prop"); + Constant *PropName = addStringToModule(Prop.first, "prop"); Constant *PropValAddr = nullptr; Constant *PropType = ConstantInt::get(Type::getInt32Ty(C), Prop.second.getType()); @@ -921,7 +921,7 @@ class BinaryWrapper { if (!Props) return Props.takeError(); // get the next the middle column element - auto *Category = addStringToModule(PropSet.first(), "SYCL_PropSetName"); + auto *Category = addStringToModule(PropSet.first, "SYCL_PropSetName"); PropSetsInits.push_back(ConstantStruct::get( getSyclPropSetTy(), Category, Props.get().first, Props.get().second)); } diff --git a/llvm/include/llvm/Support/PropertySetIO.h b/llvm/include/llvm/Support/PropertySetIO.h index dd0fb1c7f863b..93e045256ed93 100644 --- a/llvm/include/llvm/Support/PropertySetIO.h +++ b/llvm/include/llvm/Support/PropertySetIO.h @@ -33,11 +33,13 @@ #ifndef LLVM_SUPPORT_PROPERTYSETIO_H #define LLVM_SUPPORT_PROPERTYSETIO_H -#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/MapVector.h" +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Support/xxhash.h" namespace llvm { namespace util { @@ -169,18 +171,28 @@ class PropertyValue { } Val; }; -/// A property set. Doesn't preserves order of elements. -using PropertySet = StringMap; +/// Structure for specialization of DenseMap in PropertySetRegistry. +struct PropertySetKeyInfo { + static unsigned getHashValue(const SmallString<16> &K) { return xxHash64(K); } + + static SmallString<16> getEmptyKey() { return SmallString<16>(""); } + + static SmallString<16> getTombstoneKey() { return SmallString<16>("_"); } + + static bool isEqual(StringRef L, StringRef R) { return L == R; } +}; + +using PropertyMapTy = DenseMap, unsigned, PropertySetKeyInfo>; +/// A property set. Preserves insertion order when iterating elements. +using PropertySet = MapVector, PropertyValue, PropertyMapTy>; /// A registry of property sets. Maps a property set name to its /// content. /// -/// The order of keys is not preserved because Hash Map is used. -/// However, write() method prints content sorted by Category names -/// and sorted by keys. This is made only for testing, don't rely on this. +/// The order of keys is preserved and corresponds to the order of insertion. class PropertySetRegistry { public: - using MapTy = StringMap; + using MapTy = MapVector, PropertySet, PropertyMapTy>; // Specific property category names used by tools. static constexpr char SYCL_SPECIALIZATION_CONSTANTS[] = @@ -208,18 +220,6 @@ class PropertySetRegistry { PropSet.insert_or_assign(Prop.first, PropertyValue(Prop.second)); } - /// Function for bulk addition of an entire property set in the given \p - /// Category . - template - void add(StringRef Category, const StringMap &Props) { - assert(PropSetMap.find(Category) == PropSetMap.end() && - "category already added"); - auto &PropSet = PropSetMap[Category]; - - for (const auto &Prop : Props) - PropSet.insert_or_assign(Prop.first(), PropertyValue(Prop.getValue())); - } - /// Adds the given \p PropVal with the given \p PropName into the given \p /// Category . template diff --git a/llvm/lib/Support/PropertySetIO.cpp b/llvm/lib/Support/PropertySetIO.cpp index d91e18f3df397..4463e2fa2006f 100644 --- a/llvm/lib/Support/PropertySetIO.cpp +++ b/llvm/lib/Support/PropertySetIO.cpp @@ -122,23 +122,11 @@ raw_ostream &operator<<(raw_ostream &Out, const PropertyValue &Prop) { } // namespace llvm void PropertySetRegistry::write(raw_ostream &Out) const { - SmallVector Keys1(PropSetMap.size()); - for (const auto &[I, PS] : enumerate(PropSetMap)) - Keys1[I] = PS.first(); - - sort(Keys1.begin(), Keys1.end()); - for (auto K1 : Keys1) { - const auto &PropSet = PropSetMap.at(K1); - Out << "[" << K1 << "]\n"; - - SmallVector Keys2(PropSet.size()); - for (const auto &[I, Props] : enumerate(PropSet)) - Keys2[I] = Props.first(); - - sort(Keys2.begin(), Keys2.end()); - for (auto K2 : Keys2) { - const auto &Prop = PropSet.at(K2); - Out << K2 << "=" << Prop << "\n"; + for (const auto &PropSet : PropSetMap) { + Out << "[" << PropSet.first << "]\n"; + + for (const auto &Props : PropSet.second) { + Out << Props.first << "=" << Props.second << "\n"; } } } diff --git a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll index 27c395195763e..daa98b590d48a 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020-zeroinitializer-array-of-arrays.ll @@ -108,9 +108,9 @@ attributes #2 = { convergent "frame-pointer"="all" "no-trapping-math"="true" "st !9 = !{!"_ZTSN2cl4sycl14kernel_handlerE", !5, i64 0} ; CHECK-PROP: [SYCL/specialization constants] +; CHECK-PROP-NEXT: 9f47062a80eecfa7____ZL8coeff_id=2|gNAAAAAAAAAAAAAAAAAAAQAAAAQAAAAAEAAAAQAAAAgAAAAAIAAAAQAAAAwAAAAAMAAAAQAAAAABAAAAQAAAAQAAAAQBAAAAUAAAAQAAAAgBAAAAYAAAAQAAAAwBAAAAcAAAAQAAAAACAAAAgAAAAQAAAAA ; CHECK-PROP-NEXT: 405761736d5a1797____ZL9coeff_id2=2|gNAAAAAAAAQCAAAAAAAAAQAAAAgCAAAAEAAAAQAAAAwCAAAAIAAAAQAAAAADAAAAMAAAAQAAAAQDAAAAQAAAAQAAAAgDAAAAUAAAAQAAAAwDAAAAYAAAAQAAAAAEAAAAcAAAAQAAAAQEAAAAgAAAAQAAAAA ; CHECK-PROP-NEXT: 6da74a122db9f35d____ZL9coeff_id3=2|AGAAAAAAAAgEAAAAAAAAAQAAAAwEAAAAEAAAAQAAAAAFAAAAIAAAAQAAAAQFAAAAQAAAAgAAAAA -; CHECK-PROP-NEXT: 9f47062a80eecfa7____ZL8coeff_id=2|gNAAAAAAAAAAAAAAAAAAAQAAAAQAAAAAEAAAAQAAAAgAAAAAIAAAAQAAAAwAAAAAMAAAAQAAAAABAAAAQAAAAQAAAAQBAAAAUAAAAQAAAAgBAAAAYAAAAQAAAAwBAAAAcAAAAQAAAAACAAAAgAAAAQAAAAA ; CHECK-PROP: [SYCL/specialization constants default values] ; CHECK-PROP-NEXT: all=2|AMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAg/AAAAA0MzMIQzMzoAZmZGDEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA diff --git a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll index 826b8752c8b9b..8940a2e79b83f 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll @@ -266,15 +266,15 @@ attributes #3 = { nounwind } ; CHECK-RT-SAME: i32 [[#SCID17]], i32 4, i32 4} ; CHECK-PROPS: [SYCL/specialization constants] +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z9id_halfEE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z6id_intEE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z9id_composEE=2| ; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_compos2EE=2| ; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_compos3EE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_vectorEE=2| +; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marrayEE=2| ; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marray2EE=2| ; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marray3EE=2| ; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marray4EE=2| -; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_marrayEE=2| -; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z10id_vectorEE=2| -; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z6id_intEE=2| -; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z9id_composEE=2| -; CHECK-PROPS-NEXT: _ZTS14name_generatorIL_Z9id_halfEE=2| ; CHECK-PROPS-DEF: [SYCL/specialization constants default values] ; CHECK-PROPS-DEF: all=2| diff --git a/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll b/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll index 2b61600afc909..5b725728b59d8 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/SYCL2020-struct-with-undef-padding.ll @@ -83,8 +83,8 @@ attributes #4 = { convergent } ; CHECK-IR: ![[#MN3]] = !{%struct.coeff2_str_aligned_t { %"class.std::array" zeroinitializer, i64 0, [7 x i8] undef, i8 undef }} ; CHECK-PROP: [SYCL/specialization constants] -; CHECK-PROP-NEXT: df991fa0adf9bad8____ZL8coeff_id2=2| ; CHECK-PROP-NEXT: ef880fa09cf7a9d7____ZL8coeff_id=2| +; CHECK-PROP-NEXT: df991fa0adf9bad8____ZL8coeff_id2=2| ; CHECK-PROP: [SYCL/specialization constants default values] ; CHECK-PROP-NEXT: all=2| diff --git a/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll b/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll index e50490e73602a..a7e446e67dfab 100644 --- a/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll +++ b/llvm/test/tools/sycl-post-link/spec-constants/composite-default-value-padding.ll @@ -96,8 +96,8 @@ attributes #5 = { nounwind } ; Most important information from the corresponding encoded data is the size of ; the specialization constants, i.e. 8 and 1 bytes respectively. ; CHECK: [SYCL/specialization constants] -; CHECK-NEXT: 9d329ad59055e972____ZL10BoolSpecId=2|gBAAAAAAAAQAAAAAAAAAAEAAAAA ; CHECK-NEXT: 9d329ad59055e972____ZL12StructSpecId=2|gBAAAAAAAAAAAAAAAAAAAgAAAAA +; CHECK-NEXT: 9d329ad59055e972____ZL10BoolSpecId=2|gBAAAAAAAAQAAAAAAAAAAEAAAAA ; Ensure that the default values are correct. ; IBAAAAAAAAAFAAAAjBAAAEA is decoded to "0x48 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x14 diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index f0a4f59f56995..d90ae795f7cdc 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -434,8 +434,7 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, // so they won't make it into the export list. Should the check be // F->getCallingConv() != CallingConv::SPIR_KERNEL? if (F->getCallingConv() == CallingConv::SPIR_FUNC) { - PropSet[PropSetRegTy::SYCL_EXPORTED_SYMBOLS].insert( - {F->getName(), true}); + PropSet.add(PropSetRegTy::SYCL_EXPORTED_SYMBOLS, F->getName(), true); } } } @@ -444,8 +443,6 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, SmallVector MetadataNames; if (GlobProps.EmitProgramMetadata) { - auto &ProgramMetadata = PropSet[PropSetRegTy::SYCL_PROGRAM_METADATA]; - // Add reqd_work_group_size information to program metadata for (const Function &Func : M.functions()) { std::vector KernelReqdWorkGroupSize = @@ -453,8 +450,8 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, if (KernelReqdWorkGroupSize.empty()) continue; MetadataNames.push_back(Func.getName().str() + "@reqd_work_group_size"); - ProgramMetadata.insert_or_assign(MetadataNames.back(), - KernelReqdWorkGroupSize); + PropSet.add(PropSetRegTy::SYCL_PROGRAM_METADATA, MetadataNames.back(), + KernelReqdWorkGroupSize); } // Add global_id_mapping information with mapping between device-global @@ -465,12 +462,12 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, StringRef GlobalID = getGlobalVariableUniqueId(GV); MetadataNames.push_back(GlobalID.str() + "@global_id_mapping"); - ProgramMetadata.insert_or_assign(MetadataNames.back(), GV.getName()); + PropSet.add(PropSetRegTy::SYCL_PROGRAM_METADATA, MetadataNames.back(), + GV.getName()); } } if (MD.isESIMD()) { - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign("isEsimdImage", - true); + PropSet.add(PropSetRegTy::SYCL_MISC_PROP, "isEsimdImage", true); } { StringRef RegAllocModeAttr = "sycl-register-alloc-mode"; @@ -484,8 +481,8 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, return true; }); if (HasRegAllocMode) { - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert( - {RegAllocModeAttr, RegAllocModeVal}); + PropSet.add(PropSetRegTy::SYCL_MISC_PROP, RegAllocModeAttr, + RegAllocModeVal); } } @@ -501,7 +498,7 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, return true; }); if (HasGRFSize) { - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert({GRFSizeAttr, GRFSizeVal}); + PropSet.add(PropSetRegTy::SYCL_MISC_PROP, GRFSizeAttr, GRFSizeVal); } } @@ -536,18 +533,17 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, } if (OptLevel != -1) - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign("optLevel", - OptLevel); + PropSet.add(PropSetRegTy::SYCL_MISC_PROP, "optLevel", OptLevel); } { std::vector FuncNames = getKernelNamesUsingAssert(M); for (const StringRef &FName : FuncNames) - PropSet[PropSetRegTy::SYCL_ASSERT_USED].insert_or_assign(FName, true); + PropSet.add(PropSetRegTy::SYCL_ASSERT_USED, FName, true); } { if (isModuleUsingAsan(M)) - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign("asanUsed", true); + PropSet.add(PropSetRegTy::SYCL_MISC_PROP, "asanUsed", true); } if (GlobProps.EmitDeviceGlobalPropSet) { @@ -563,8 +559,8 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, } if (MD.isSpecConstantDefault()) - PropSet[PropSetRegTy::SYCL_MISC_PROP].insert_or_assign( - "specConstsReplacedWithDefault", 1); + PropSet.add(PropSetRegTy::SYCL_MISC_PROP, "specConstsReplacedWithDefault", + 1); std::error_code EC; std::string SCFile = makeResultFileName(".prop", I, Suff); diff --git a/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp b/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp index ca86ff3c213ed..0aefe26bd826c 100644 --- a/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp +++ b/sycl/test/basic_tests/SYCL-2020-spec-const-ids-order.cpp @@ -46,10 +46,10 @@ int main() { } // CHECK-PROP: [SYCL/specialization constants] -// CHECK-PROP-NEXT: [[UNIQUE_PREFIX:[a-z0-9]+]]____ZL10ConstantId +// CHECK-PROP-NEXT: [[UNIQUE_PREFIX:[a-z0-9]+]]____ZL5Val23 +// CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL10ConstantId // CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL11SecondValue // CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL11SpecConst42 -// CHECK-PROP-NEXT: [[UNIQUE_PREFIX]]____ZL5Val23 // CHECK-IR: !sycl.specialization-constants = !{![[#MD0:]], ![[#MD1:]], ![[#MD2:]], ![[#MD3:]]} // CHECK-IR: ![[#MD0]] = !{!"[[UNIQUE_PREFIX:[a-z0-9]+]]____ZL5Val23", i32 [[#ID:]]