perf(rocr): shadow XGMI device batches, read timestamps with one load

projects/rocr-runtime/runtime/hsa-runtime/core/inc/signal.h

@@ -68,6 +68,10 @@
 #include "inc/amd_hsa_signal.h"
 #include "inc/hsa_ext_amd.h"
 
+#if defined(__x86_64__) || defined(_M_X64)
+#include <emmintrin.h>
+#endif
+
 #if defined(__i386__) || defined(__x86_64__)
 #include <mwaitxintrin.h>
 #ifndef MWAITX_ECX_TIMER_ENABLE
@@ -176,8 +180,23 @@ struct SharedSignal {
   }
 
   void GetRawTs(uint64_t& start, uint64_t& end) const {
+#if defined(__x86_64__) || defined(_M_X64)
+    // start_ts and end_ts fill one aligned 16-byte chunk, which a single load reads in one round
+    // trip when the signal is in device memory; two loads would cost two
+    static_assert(offsetof(amd_signal_t, start_ts) % 16 == 0 &&
+                      offsetof(amd_signal_t, end_ts) == offsetof(amd_signal_t, start_ts) + 8,
+                  "start_ts and end_ts must fill one aligned 16-byte chunk");
+    // The load faults on a misaligned address. The signal allocators align to SharedSignal.
+    static_assert(alignof(amd_signal_t) >= 16 && offsetof(SharedSignal, amd_signal) % 16 == 0,
+                  "amd_signal must be 16-byte aligned in every SharedSignal");
+    const __m128i timestamps =
+        _mm_load_si128(reinterpret_cast<const __m128i*>(&amd_signal.start_ts));
+    start = _mm_cvtsi128_si64(timestamps);
+    end = _mm_cvtsi128_si64(_mm_unpackhi_epi64(timestamps, timestamps));
+#else
     start = amd_signal.start_ts;
     end = amd_signal.end_ts;
+#endif
   }
 
   static __forceinline SharedSignal* Convert(hsa_signal_t signal) {
@@ -237,12 +256,12 @@ void FlushDeviceStores(volatile int64_t* stored, int64_t value);
 class SignalBatch {
  public:
   /// @p access_agents are granted access to the arena; empty when @p region is
-  /// already mapped for every agent.
+  /// already mapped for every agent. @p shadow_members keeps a host-side copy of each member's
+  /// Signal*, for an arena the host reaches across a link.
   static hsa_status_t Create(uint32_t count, const hsa_signal_value_t* initial_values,
                              bool use_interrupt, const MemoryRegion* region,
                              const std::vector<hsa_agent_t>& access_agents,
-                             bool host_access_via_hdp,
-                             SignalBatch** batch,
+                             bool host_access_via_hdp, bool shadow_members, SignalBatch** batch,
                              hsa_signal_t* signals);
 
   static __forceinline hsa_amd_signal_batch_t Convert(SignalBatch* batch) {
@@ -270,10 +289,10 @@ class SignalBatch {
     return num_shadowed_arenas_.load(std::memory_order_relaxed) != 0;
   }
 
-  /// @brief Resolves @p handle from host memory when it names a slot of an arena the host
-  /// reaches across the bus, where reading the slot's id and core_signal would cost a bus round
-  /// trip each. Returns false when @p handle is in no such arena; otherwise sets @p member,
-  /// to nullptr when the member has been destroyed.
+  /// @brief Resolves @p handle from host memory when it names a slot of an arena in device
+  /// memory, which the host reaches across PCIe or XGMI, so reading the slot's id and
+  /// core_signal would cost a round trip each. Returns false when @p handle is in no such arena;
+  /// otherwise sets @p member, to nullptr when the member has been destroyed.
   static bool FindShadowedMember(uint64_t handle, Signal** member);
 
  private:
@@ -302,7 +321,7 @@ class SignalBatch {
   const bool host_access_via_hdp_;
 
   /// Host copy of each slot's core_signal, cleared when the member is destroyed. Only
-  /// allocated for arenas the host reaches across the bus.
+  /// allocated for arenas in device memory.
   std::unique_ptr<std::atomic<Signal*>[]> shadow_members_;
 
   /// One for the batch handle plus one per live member object.

projects/rocr-runtime/runtime/hsa-runtime/core/runtime/hsa_ext_amd.cpp

@@ -1101,8 +1101,13 @@ hsa_status_t hsa_amd_signal_batch_create(uint32_t count,
     return static_cast<hsa_status_t>(HSA_STATUS_ERROR_INVALID_MEMORY_POOL);
   }
 
-  const bool host_access_via_hdp =
-      device_pool && static_cast<const AMD::GpuAgent*>(region->owner())->PcieStoresNeedFlush();
+  const auto* owner_gpu =
+      device_pool ? static_cast<const AMD::GpuAgent*>(region->owner()) : nullptr;
+  const bool host_access_via_hdp = device_pool && owner_gpu->PcieStoresNeedFlush();
+  // The host reaches a discrete GPU's memory across PCIe or XGMI, whether or not its stores need
+  // an HDP flush, so resolving a member through its slot would cost link round trips. An APU's
+  // device pool is in memory the host shares, where reading the slot is as cheap as the shadow.
+  const bool shadow_members = device_pool && !owner_gpu->properties().Integrated;
 
   bool use_interrupt;
   hsa_status_t status =
@@ -1124,7 +1129,7 @@ hsa_status_t hsa_amd_signal_batch_create(uint32_t count,
 
   core::SignalBatch* signal_batch;
   status = core::SignalBatch::Create(count, initial_values, use_interrupt, region, access_agents,
-                                     host_access_via_hdp, &signal_batch, signals);
+                                     host_access_via_hdp, shadow_members, &signal_batch, signals);
   if (status != HSA_STATUS_SUCCESS) return status;
 
   *batch = core::SignalBatch::Convert(signal_batch);

projects/rocr-runtime/runtime/hsa-runtime/core/runtime/signal.cpp

@@ -508,8 +508,7 @@ int64_t Signal::UpdateValueWithoutRmw(ValueOp op, int64_t operand, int64_t expec
 hsa_status_t SignalBatch::Create(uint32_t count, const hsa_signal_value_t* initial_values,
                                  bool use_interrupt, const MemoryRegion* region,
                                  const std::vector<hsa_agent_t>& access_agents,
-                                 bool host_access_via_hdp,
-                                 SignalBatch** batch,
+                                 bool host_access_via_hdp, bool shadow_members, SignalBatch** batch,
                                  hsa_signal_t* signals) {
   const size_t arena_bytes = size_t(count) * sizeof(SharedSignal);
 
@@ -529,7 +528,7 @@ hsa_status_t SignalBatch::Create(uint32_t count, const hsa_signal_value_t* initi
 
   SignalBatch* new_batch = new SignalBatch(arena, count, host_access_via_hdp);
   arenaGuard.Dismiss();
-  if (host_access_via_hdp) {
+  if (shadow_members) {
     new_batch->shadow_members_.reset(new std::atomic<Signal*>[count]());
     new_batch->RegisterShadow();
   }