[SYCL][NFCI] Refactor unittest::UrArray (#15604)

This is a first patch in series of detaching unit-test classes from UR
names.

In a previous attempts to do so (#14815) there were concerns about what
to do with `UrArray` and this PR is another attempt (see also #15014) to
do that.

This PR renames `UrArray` into `LifetimeExtender` to better communicate
its purpose: data structures emitted by the compiler (and therefore used
by the runtime) to describe device image and their properties do not
store data, but only hold pointers to them. Therefore when we mock those
in our unit-tests we need to ensure that their lifetime is long enough
to cover the whole test.

This is a non-functional change by its spirit, but what used to be
`UrArray` is now hidden from writers of unit-tests and the interface is
switched to `std::vector` - that is done to hide an implementation
detail and simplify amount of knowledge required to write unit-tests.

Author: AlexeySachkov

sycl/unittests/Extensions/DeviceGlobal.cpp

@@ -65,11 +65,11 @@ static sycl::unittest::UrImage generateDeviceGlobalImage() {
   UrProperty DevGlobInfo =
       makeDeviceGlobalInfo(DeviceGlobalName, sizeof(int) * 2, 0);
   PropSet.insert(__SYCL_PROPERTY_SET_SYCL_DEVICE_GLOBALS,
-                 UrArray<UrProperty>{std::move(DevGlobInfo)});
+                 std::vector<UrProperty>{std::move(DevGlobInfo)});
 
   std::vector<unsigned char> Bin{10, 11, 12, 13, 14, 15}; // Random data
 
-  UrArray<UrOffloadEntry> Entries =
+  std::vector<UrOffloadEntry> Entries =
       makeEmptyKernels({DeviceGlobalTestKernelName});
 
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format
@@ -96,11 +96,11 @@ static sycl::unittest::UrImage generateDeviceGlobalImgScopeImage() {
   UrProperty DevGlobInfo =
       makeDeviceGlobalInfo(DeviceGlobalImgScopeName, sizeof(int) * 2, 1);
   PropSet.insert(__SYCL_PROPERTY_SET_SYCL_DEVICE_GLOBALS,
-                 UrArray<UrProperty>{std::move(DevGlobInfo)});
+                 std::vector<UrProperty>{std::move(DevGlobInfo)});
 
   std::vector<unsigned char> Bin{10, 11, 12, 13, 14, 15}; // Random data
 
-  UrArray<UrOffloadEntry> Entries =
+  std::vector<UrOffloadEntry> Entries =
       makeEmptyKernels({DeviceGlobalImgScopeTestKernelName});
 
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format

sycl/unittests/Extensions/USMMemcpy2D.cpp

@@ -132,7 +132,7 @@ static sycl::unittest::UrImage generateMemopsImage() {
 
   std::vector<unsigned char> Bin{10, 11, 12, 13, 14, 15}; // Random data
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels(
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels(
       {USMFillHelperKernelNameLong, USMFillHelperKernelNameChar,
        USMMemcpyHelperKernelNameLong, USMMemcpyHelperKernelNameChar});
 

sycl/unittests/Extensions/VirtualFunctions/RuntimeLinking.cpp

@@ -50,7 +50,7 @@ static sycl::unittest::UrImage
 generateImage(std::initializer_list<std::string> KernelNames,
               const std::string &VFSets, bool UsesVFSets, unsigned char Magic) {
   sycl::unittest::UrPropertySet PropSet;
-  sycl::unittest::UrArray<sycl::unittest::UrProperty> Props;
+  std::vector<sycl::unittest::UrProperty> Props;
   uint64_t PropSize = VFSets.size();
   std::vector<char> Storage(/* bytes for size */ 8 + PropSize +
                             /* null terminator */ 1);
@@ -69,7 +69,7 @@ generateImage(std::initializer_list<std::string> KernelNames,
 
   std::vector<unsigned char> Bin{Magic};
 
-  sycl::unittest::UrArray<sycl::unittest::UrOffloadEntry> Entries =
+  std::vector<sycl::unittest::UrOffloadEntry> Entries =
       sycl::unittest::makeEmptyKernels(KernelNames);
 
   sycl::unittest::UrImage Img{

sycl/unittests/SYCL2020/IsCompatible.cpp

@@ -32,7 +32,7 @@ generateDefaultImage(std::initializer_list<std::string> KernelNames,
 
   std::vector<unsigned char> Bin{0, 1, 2, 3, 4, 5}; // Random data
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
 
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format
               __SYCL_DEVICE_BINARY_TARGET_SPIRV64, // DeviceTargetSpec

sycl/unittests/SYCL2020/KernelBundle.cpp

@@ -37,7 +37,7 @@ generateDefaultImage(std::initializer_list<std::string> KernelNames,
 
   std::vector<unsigned char> Bin{0, 1, 2, 3, 4, 5}; // Random data
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
 
   UrImage Img{BinaryType, // Format
               DeviceTargetSpec,

sycl/unittests/SYCL2020/KernelBundleStateFiltering.cpp

@@ -46,7 +46,7 @@ generateDefaultImage(std::initializer_list<std::string> KernelNames,
   static unsigned char NImage = 0;
   std::vector<unsigned char> Bin{NImage++};
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
 
   UrImage Img{BinaryType, // Format
               DeviceTargetSpec,

sycl/unittests/SYCL2020/KernelID.cpp

@@ -55,7 +55,7 @@ generateDefaultImage(std::initializer_list<std::string> Kernels) {
 
   std::vector<unsigned char> Bin{0, 1, 2, 3, 4, 5}; // Random data
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels(Kernels);
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels(Kernels);
 
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format
               __SYCL_DEVICE_BINARY_TARGET_SPIRV64, // DeviceTargetSpec

sycl/unittests/SYCL2020/SpecializationConstant.cpp

@@ -49,7 +49,7 @@ static sycl::unittest::UrImage generateImageWithSpecConsts() {
 
   std::vector<unsigned char> Bin{0, 1, 2, 3, 4, 5}; // Random data
 
-  UrArray<UrOffloadEntry> Entries =
+  std::vector<UrOffloadEntry> Entries =
       makeEmptyKernels({"SpecializationConstant_TestKernel"});
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format
               __SYCL_DEVICE_BINARY_TARGET_SPIRV64, // DeviceTargetSpec

sycl/unittests/assert/assert.cpp

@@ -86,7 +86,7 @@ static sycl::unittest::UrImage generateDefaultImage() {
 
   std::vector<unsigned char> Bin{0, 1, 2, 3, 4, 5}; // Random data
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels({KernelName});
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels({KernelName});
 
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format
               __SYCL_DEVICE_BINARY_TARGET_SPIRV64, // DeviceTargetSpec
@@ -109,7 +109,7 @@ static sycl::unittest::UrImage generateCopierKernelImage() {
 
   std::vector<unsigned char> Bin{10, 11, 12, 13, 14, 15}; // Random data
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels({CopierKernelName});
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels({CopierKernelName});
 
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format
               __SYCL_DEVICE_BINARY_TARGET_SPIRV64, // DeviceTargetSpec

sycl/unittests/helpers/UrImage.hpp

@@ -109,62 +109,49 @@ class UrOffloadEntry {
   NativeType MNative;
 };
 
-/// Generic array of UR entries.
-template <typename T> class UrArray {
+namespace internal {
+// Content from this namespace shouldn't be used anywhere outside of this file
+
+/// "native" data structures used by SYCL RT do not hold the data, but only
+/// point to it. The data itself is embedded into the binary data sections in
+/// real applications.
+/// In unit-tests we mock those data structures and therefore we need to ensure
+/// that the lifetime of the underlying data is correct and we won't perform
+/// any illegal memory accesses in unit-tests.
+template <typename T> class LifetimeExtender {
 public:
-  explicit UrArray(std::vector<T> Entries) : MMockEntries(std::move(Entries)) {
-    updateEntries();
-  }
-
-  UrArray(std::initializer_list<T> Entries) : MMockEntries(std::move(Entries)) {
-    updateEntries();
+  explicit LifetimeExtender(std::vector<T> Entries)
+      : MMockEntries(std::move(Entries)) {
+    MEntries.clear();
+    std::transform(MMockEntries.begin(), MMockEntries.end(),
+                   std::back_inserter(MEntries),
+                   [](const T &Entry) { return Entry.convertToNativeType(); });
   }
 
-  UrArray() = default;
-
-  void push_back(const T &Entry) {
-    MMockEntries.push_back(Entry);
-    MEntriesNeedUpdate = true;
-  }
+  LifetimeExtender() = default;
 
   typename T::NativeType *begin() {
-    if (MEntriesNeedUpdate) {
-      updateEntries();
-    }
-
     if (MEntries.empty())
       return nullptr;
 
     return &*MEntries.begin();
   }
   typename T::NativeType *end() {
-    if (MEntriesNeedUpdate) {
-      updateEntries();
-    }
-
     if (MEntries.empty())
       return nullptr;
 
     return &*MEntries.rbegin() + 1;
   }
 
 private:
-  void updateEntries() {
-    MEntries.clear();
-    std::transform(MMockEntries.begin(), MMockEntries.end(),
-                   std::back_inserter(MEntries),
-                   [](const T &Entry) { return Entry.convertToNativeType(); });
-  }
   std::vector<T> MMockEntries;
   std::vector<typename T::NativeType> MEntries;
-  bool MEntriesNeedUpdate = false;
 };
 
 #ifdef __cpp_deduction_guides
-template <typename T> UrArray(std::vector<T>) -> UrArray<T>;
-
-template <typename T> UrArray(std::initializer_list<T>) -> UrArray<T>;
+template <typename T> LifetimeExtender(std::vector<T>) -> LifetimeExtender<T>;
 #endif // __cpp_deduction_guides
+} // namespace internal
 
 /// Convenience wrapper for sycl_device_binary_property_set.
 class UrPropertySet {
@@ -187,19 +174,23 @@ class UrPropertySet {
     // Value must be an all-zero 32-bit mask, which would mean that no fallback
     // libraries are needed to be loaded.
     UrProperty DeviceLibReqMask("", Data, SYCL_PROPERTY_TYPE_UINT32);
-    insert(__SYCL_PROPERTY_SET_DEVICELIB_REQ_MASK, UrArray{DeviceLibReqMask});
+    insert(__SYCL_PROPERTY_SET_DEVICELIB_REQ_MASK, std::move(DeviceLibReqMask));
+  }
+
+  /// Adds a new property to the set.
+  ///
+  /// \param Name is a property name. See ur.hpp for list of known names.
+  /// \param Prop is a property value.
+  void insert(const std::string &Name, UrProperty &&Props) {
+    insert(Name, internal::LifetimeExtender{std::vector{std::move(Props)}});
   }
 
   /// Adds a new array of properties to the set.
   ///
   /// \param Name is a property array name. See ur.hpp for list of known names.
   /// \param Props is an array of property values.
-  void insert(const std::string &Name, UrArray<UrProperty> Props) {
-    MNames.push_back(Name);
-    MMockProperties.push_back(std::move(Props));
-    MProperties.push_back(_sycl_device_binary_property_set_struct{
-        MNames.back().data(), MMockProperties.back().begin(),
-        MMockProperties.back().end()});
+  void insert(const std::string &Name, std::vector<UrProperty> &&Props) {
+    insert(Name, internal::LifetimeExtender{std::move(Props)});
   }
 
   _sycl_device_binary_property_set_struct *begin() {
@@ -215,36 +206,65 @@ class UrPropertySet {
   }
 
 private:
+  /// Adds a new array of properties to the set.
+  ///
+  /// \param Name is a property array name. See ur.hpp for list of known names.
+  /// \param Props is an array of property values.
+  void insert(const std::string &Name,
+              internal::LifetimeExtender<UrProperty> Props) {
+    MNames.push_back(Name);
+    MMockProperties.push_back(std::move(Props));
+    MProperties.push_back(_sycl_device_binary_property_set_struct{
+        MNames.back().data(), MMockProperties.back().begin(),
+        MMockProperties.back().end()});
+  }
+
   std::vector<std::string> MNames;
-  std::vector<UrArray<UrProperty>> MMockProperties;
+  std::vector<internal::LifetimeExtender<UrProperty>> MMockProperties;
   std::vector<_sycl_device_binary_property_set_struct> MProperties;
 };
 
 /// Convenience wrapper around UR internal structures, that manages UR binary
 /// image data lifecycle.
 class UrImage {
-public:
+private:
   /// Constructs an arbitrary device image.
   UrImage(uint16_t Version, uint8_t Kind, uint8_t Format,
           const std::string &DeviceTargetSpec,
           const std::string &CompileOptions, const std::string &LinkOptions,
-          std::vector<char> Manifest, std::vector<unsigned char> Binary,
-          UrArray<UrOffloadEntry> OffloadEntries, UrPropertySet PropertySet)
+          std::vector<char> &&Manifest, std::vector<unsigned char> &&Binary,
+          internal::LifetimeExtender<UrOffloadEntry> OffloadEntries,
+          UrPropertySet PropertySet)
       : MVersion(Version), MKind(Kind), MFormat(Format),
         MDeviceTargetSpec(DeviceTargetSpec), MCompileOptions(CompileOptions),
         MLinkOptions(LinkOptions), MManifest(std::move(Manifest)),
         MBinary(std::move(Binary)), MOffloadEntries(std::move(OffloadEntries)),
         MPropertySet(std::move(PropertySet)) {}
 
+public:
+  /// Constructs an arbitrary device image.
+  UrImage(uint16_t Version, uint8_t Kind, uint8_t Format,
+          const std::string &DeviceTargetSpec,
+          const std::string &CompileOptions, const std::string &LinkOptions,
+          std::vector<char> &&Manifest, std::vector<unsigned char> &&Binary,
+          std::vector<UrOffloadEntry> &&OffloadEntries,
+          UrPropertySet PropertySet)
+      : UrImage(Version, Kind, Format, DeviceTargetSpec, CompileOptions,
+                LinkOptions, std::move(Manifest), std::move(Binary),
+                internal::LifetimeExtender(std::move(OffloadEntries)),
+                std::move(PropertySet)) {}
+
   /// Constructs a SYCL device image of the latest version.
   UrImage(uint8_t Format, const std::string &DeviceTargetSpec,
           const std::string &CompileOptions, const std::string &LinkOptions,
-          std::vector<unsigned char> Binary,
-          UrArray<UrOffloadEntry> OffloadEntries, UrPropertySet PropertySet)
+          std::vector<unsigned char> &&Binary,
+          std::vector<UrOffloadEntry> &&OffloadEntries,
+          UrPropertySet PropertySet)
       : UrImage(SYCL_DEVICE_BINARY_VERSION,
                 SYCL_DEVICE_BINARY_OFFLOAD_KIND_SYCL, Format, DeviceTargetSpec,
                 CompileOptions, LinkOptions, {}, std::move(Binary),
-                std::move(OffloadEntries), std::move(PropertySet)) {}
+                internal::LifetimeExtender(std::move(OffloadEntries)),
+                std::move(PropertySet)) {}
 
   sycl_device_binary_struct convertToNativeType() {
     return sycl_device_binary_struct{
@@ -275,7 +295,7 @@ class UrImage {
   std::string MLinkOptions;
   std::vector<char> MManifest;
   std::vector<unsigned char> MBinary;
-  UrArray<UrOffloadEntry> MOffloadEntries;
+  internal::LifetimeExtender<UrOffloadEntry> MOffloadEntries;
   UrPropertySet MPropertySet;
 };
 
@@ -392,7 +412,7 @@ inline UrProperty makeSpecConstant(std::vector<char> &ValData,
 /// Utility function to mark kernel as the one using assert
 inline void setKernelUsesAssert(const std::vector<std::string> &Names,
                                 UrPropertySet &Set) {
-  UrArray<UrProperty> Value;
+  std::vector<UrProperty> Value;
   for (const std::string &N : Names)
     Value.push_back({N, {0, 0, 0, 0}, SYCL_PROPERTY_TYPE_UINT32});
   Set.insert(__SYCL_PROPERTY_SET_SYCL_ASSERT_USED, std::move(Value));
@@ -401,16 +421,14 @@ inline void setKernelUsesAssert(const std::vector<std::string> &Names,
 /// Utility function to add specialization constants to property set.
 ///
 /// This function overrides the default spec constant values.
-inline void addSpecConstants(UrArray<UrProperty> SpecConstants,
+inline void addSpecConstants(std::vector<UrProperty> &&SpecConstants,
                              std::vector<char> ValData, UrPropertySet &Props) {
   Props.insert(__SYCL_PROPERTY_SET_SPEC_CONST_MAP, std::move(SpecConstants));
 
   UrProperty Prop{"all", std::move(ValData), SYCL_PROPERTY_TYPE_BYTE_ARRAY};
 
-  UrArray<UrProperty> DefaultValues{std::move(Prop)};
-
   Props.insert(__SYCL_PROPERTY_SET_SPEC_CONST_DEFAULT_VALUES_MAP,
-               std::move(DefaultValues));
+               std::move(Prop));
 }
 
 /// Utility function to add ESIMD kernel flag to property set.
@@ -419,15 +437,13 @@ inline void addESIMDFlag(UrPropertySet &Props) {
   ValData[0] = 1;
   UrProperty Prop{"isEsimdImage", ValData, SYCL_PROPERTY_TYPE_UINT32};
 
-  UrArray<UrProperty> Value{std::move(Prop)};
-
-  Props.insert(__SYCL_PROPERTY_SET_SYCL_MISC_PROP, std::move(Value));
+  Props.insert(__SYCL_PROPERTY_SET_SYCL_MISC_PROP, std::move(Prop));
 }
 
 /// Utility function to generate offload entries for kernels without arguments.
-inline UrArray<UrOffloadEntry>
+inline std::vector<UrOffloadEntry>
 makeEmptyKernels(std::initializer_list<std::string> KernelNames) {
-  UrArray<UrOffloadEntry> Entries;
+  std::vector<UrOffloadEntry> Entries;
 
   for (const auto &Name : KernelNames) {
     UrOffloadEntry E{Name, {}, 0};
@@ -531,7 +547,7 @@ inline void
 addDeviceRequirementsProps(UrPropertySet &Props,
                            const std::vector<sycl::aspect> &Aspects,
                            const std::vector<int> &ReqdWGSize = {}) {
-  UrArray<UrProperty> Value{makeAspectsProp(Aspects)};
+  std::vector<UrProperty> Value{makeAspectsProp(Aspects)};
   if (!ReqdWGSize.empty())
     Value.push_back(makeReqdWGSizeProp(ReqdWGSize));
   Props.insert(__SYCL_PROPERTY_SET_SYCL_DEVICE_REQUIREMENTS, std::move(Value));
@@ -550,7 +566,7 @@ generateDefaultImage(std::initializer_list<std::string> KernelNames) {
   std::vector<unsigned char> Bin(Combined.begin(), Combined.end());
   Bin.push_back(0);
 
-  UrArray<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
+  std::vector<UrOffloadEntry> Entries = makeEmptyKernels(KernelNames);
 
   UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV,       // Format
               __SYCL_DEVICE_BINARY_TARGET_SPIRV64, // DeviceTargetSpec