[llvm] [offload][l0] Implement context groups (PR #217562)

[Jan Trusiłło via llvm-commits]

================
@@ -55,37 +55,23 @@ struct OffloadTopology {
   /// \returns backend of this topology.
   ol_platform_backend_t getBackend() const { return MBackend; }
 
-  /// Returns all platforms associated with this topology.
+  /// Returns all platform context groups associated with this topology.
   ///
-  /// \returns minimal span-like view to platforms associated with this
-  /// topology.
-  range_view<const ol_platform_handle_t> getPlatforms() const;
+  /// \returns platform context groups associated with this topology.
+  const std::vector<OffloadPlatformGroup> &getPlatformGroups() const {
+    return MPlatformGroups;
+  }
 
-  /// Returns all devices associated with specific platform.
-  ///
-  /// \param PlatformId is index into MPlatforms.
-  ///
-  /// \returns minimal span-like view to devices associated with specified
-  /// platform.
-  range_view<ol_device_handle_t> getDevices(size_t PlatformId) const;
-
-  /// Register new platform and devices into this topology.
+  /// Registers platform context groups and devices into this topology.
   ///
   /// \param PlatformsAndDev collection of platforms & devices.
-  void registerNewPlatformsAndDevices(Platform2DevContainer &PlatformsAndDev);
+  void
+  registerNewPlatformsAndDevices(const Platform2DevContainer &PlatformsAndDev);
 
 private:
   ol_platform_backend_t MBackend = OL_PLATFORM_BACKEND_UNKNOWN;
 
-  // Platforms and devices belonging to this backend (flattened)
-  std::vector<ol_platform_handle_t> MPlatforms;
-
-  // Devices are sorted by platform (guarantee from liboffload)
-  std::vector<ol_device_handle_t> MDevices;
-
-  // Vector holding range of devices for each platform (index is platform index
-  // within Platforms), so MDeviceRange.size() == MPlatforms.size()
-  std::vector<range_view<ol_device_handle_t>> MDeviceRange;
+  std::vector<OffloadPlatformGroup> MPlatformGroups;
----------------
311Volt wrote:

Ideally you'd want `olIterateDevices` to guarantee an order where the context group index is non-decreasing, but the tricky part is that the L0 plugin sorts devices to have discrete devices at the front of the list (L0Plugin.cpp:92):

```cpp
  std::sort(RootDevices.begin(), RootDevices.end(),
            [](const RootInfoTy &A, const RootInfoTy &B) {
              // If both are discrete, order by OrderId.
              // If both are not discrete, order by OrderId.
              // Otherwise, discrete goes first.

/* ... */
```

As far as I can tell there's no guarantee for driver instances to contain homogenous sets of devices (as in, all discrete or all integrated), so you can end up with non-monotonic context group indices, and this order is visible in `olIterateDevices`.

I believe this is because libomptarget wants device 0 (used by default if you don't go out of your way to change it) to be a discrete one if possible. I think it would be a lot cleaner if the L0 plugin simply respected `zeDriverGet`+ `zeDeviceGet` order, and then libomptarget could do the remapping internally. This way:
 - context group indices could be specified to be monotonic
 - libsycl can have per-platform `range_view`s of devices, and a flat per-backend one for SYCL selectors
 - env vars like `ZE_AFFINITY_MASK` or `ZE_ENABLE_PCI_ID_DEVICE_ORDER` translate into liboffload (and by extension sycl) predictably

I think the fact that this touches `libomptarget` makes this worthy of another PR, though - for now, I'd either keep this the way it is, or add a lookup table to at least get rid of the linear lookup. Please share your thoughts

https://github.com/llvm/llvm-project/pull/217562