https://github.com/adurang updated https://github.com/llvm/llvm-project/pull/221764
>From e2f3f69e585c8572e09d0a9908a8ea9051eb7512 Mon Sep 17 00:00:00 2001 From: "Duran, Alex" <[email protected]> Date: Mon, 7 Sep 2026 08:35:41 -0700 Subject: [PATCH] [offload][omp] Remove data_fence All supported backends execute enqueued work on a given queue in submission order (CUDA, AMDGPU, and Host queues are always in-order; Level Zero's default and non-default in-order/synchronous command modes are as well), so the explicit data-fence used to order a pointer-attachment after prior data transfers is unnecessary. Remove the DeviceTy/GenericDeviceTy/GenericPluginTy dataFence chain and the now-unused olQueueBarrier liboffload API added to support it. --- offload/include/device.h | 4 ---- offload/liboffload/exports | 1 - offload/libompaccsupport/device.cpp | 4 ---- offload/libomptarget/omptarget.cpp | 21 ++++--------------- offload/plugins-nextgen/amdgpu/src/rtl.cpp | 7 ------- .../common/include/PluginInterface.h | 8 ------- .../common/src/PluginInterface.cpp | 12 ----------- offload/plugins-nextgen/cuda/src/rtl.cpp | 7 ------- offload/plugins-nextgen/host/src/rtl.cpp | 7 ------- .../level_zero/include/L0Device.h | 1 - .../level_zero/src/L0Device.cpp | 8 ------- 11 files changed, 4 insertions(+), 76 deletions(-) diff --git a/offload/include/device.h b/offload/include/device.h index 58b8198a4f9ac..8d6ed8ed31821 100644 --- a/offload/include/device.h +++ b/offload/include/device.h @@ -109,10 +109,6 @@ struct DeviceTy { int32_t dataExchange(void *SrcPtr, DeviceTy &DstDev, void *DstPtr, int64_t Size, AsyncInfoTy &AsyncInfo); - // Insert a data fence between previous data operations and the following - // operations if necessary for the device. - int32_t dataFence(AsyncInfoTy &AsyncInfo); - /// Notify the plugin about a new mapping starting at the host address /// \p HstPtr and \p Size bytes. int32_t notifyDataMapped(void *HstPtr, int64_t Size); diff --git a/offload/liboffload/exports b/offload/liboffload/exports index 21efd24dc3021..a1046be73e320 100644 --- a/offload/liboffload/exports +++ b/offload/liboffload/exports @@ -12,7 +12,6 @@ global: "llvm::omp::target::plugin::GenericPluginTy::create_interop(int, int, interop_spec_t*)"; "llvm::omp::target::plugin::GenericPluginTy::data_alloc(int, long, void*, int)"; "llvm::omp::target::plugin::GenericPluginTy::data_delete(int, void*, int)"; - "llvm::omp::target::plugin::GenericPluginTy::data_fence(int, __tgt_async_info*)"; "llvm::omp::target::plugin::GenericPluginTy::data_lock(int, void*, long, void**)"; "llvm::omp::target::plugin::GenericPluginTy::data_notify_mapped(int, void*, long)"; "llvm::omp::target::plugin::GenericPluginTy::data_notify_unmapped(int, void*)"; diff --git a/offload/libompaccsupport/device.cpp b/offload/libompaccsupport/device.cpp index 94d9d701d1019..583773fcee091 100644 --- a/offload/libompaccsupport/device.cpp +++ b/offload/libompaccsupport/device.cpp @@ -366,10 +366,6 @@ int32_t DeviceTy::dataExchange(void *SrcPtr, DeviceTy &DstDev, void *DstPtr, return OFFLOAD_SUCCESS; } -int32_t DeviceTy::dataFence(AsyncInfoTy &AsyncInfo) { - return RTL->data_fence(RTLDeviceID, AsyncInfo); -} - int32_t DeviceTy::notifyDataMapped(void *HstPtr, int64_t Size) { ODBG(ODT_Mapping) << "Notifying about new mapping: HstPtr=" << HstPtr << ", Size=" << Size; diff --git a/offload/libomptarget/omptarget.cpp b/offload/libomptarget/omptarget.cpp index f5343fe61dd49..24a9b93d1a0ea 100644 --- a/offload/libomptarget/omptarget.cpp +++ b/offload/libomptarget/omptarget.cpp @@ -832,10 +832,10 @@ int targetDataBegin(ident_t *Loc, DeviceTy &Device, int32_t ArgNum, /// /// (4) and (1) are both trying to modify the device memory corresponding to /// `&p`. So, if we decide that (4) should do an attachment, we also need to -/// ensure that (4) happens after (1) is complete. -/// -/// For this purpose, we insert a data_fence before the first -/// pointer-attachment, (3), to ensure that all pending transfers finish first. +/// ensure that (4) happens after (1) is complete. Since all supported +/// backends execute enqueued work on a given queue in submission order, this +/// is already guaranteed as long as (1) and (4) are submitted to the same +/// queue, without requiring an explicit fence. int processAttachEntries(DeviceTy &Device, StateInfoTy &StateInfo, AsyncInfoTy &AsyncInfo) { // Report all tracked allocations from both main loop and ATTACH processing @@ -862,7 +862,6 @@ int processAttachEntries(DeviceTy &Device, StateInfoTy &StateInfo, << "LIBOMPTARGET_TREAT_ATTACH_AUTO_AS_ALWAYS is true"; int Ret = OFFLOAD_SUCCESS; - bool IsFirstPointerAttachment = true; for (size_t EntryIdx = 0; EntryIdx < StateInfo.AttachEntries.size(); ++EntryIdx) { const auto &AttachEntry = StateInfo.AttachEntries[EntryIdx]; @@ -951,18 +950,6 @@ int processAttachEntries(DeviceTy &Device, StateInfoTy &StateInfo, TargetPointerResultTy &PtrTPR = *PtrTPROpt; void **TgtPtrBase = reinterpret_cast<void **>(PtrTPR.TargetPointer); - // Insert a data-fence before the first pointer-attachment. - if (IsFirstPointerAttachment) { - IsFirstPointerAttachment = false; - ODBG(ODT_Mapping) - << "Inserting a data fence before the first pointer attachment."; - Ret = Device.dataFence(AsyncInfo); - if (Ret != OFFLOAD_SUCCESS) { - REPORT() << "Failed to insert data fence."; - return OFFLOAD_FAIL; - } - } - // Do the pointer-attachment, i.e. update the device pointer to point to // device pointee. Ret = performPointerAttachment(Device, AsyncInfo, HstPtr, HstPteeBase, diff --git a/offload/plugins-nextgen/amdgpu/src/rtl.cpp b/offload/plugins-nextgen/amdgpu/src/rtl.cpp index 281b9e3795a54..7671a6ff79bee 100644 --- a/offload/plugins-nextgen/amdgpu/src/rtl.cpp +++ b/offload/plugins-nextgen/amdgpu/src/rtl.cpp @@ -3023,13 +3023,6 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy { getAgent(), (uint64_t)Size); } - /// Insert a data fence between previous data operations and the following - /// operations. This is a no-op for AMDGPU devices as operations inserted into - /// a queue are in-order. - Error dataFence(__tgt_async_info *Async) override { - return Plugin::success(); - } - Error dataFillImpl(void *TgtPtr, const void *PatternPtr, int64_t PatternSize, int64_t Size, AsyncInfoWrapperTy &AsyncInfoWrapper) override { diff --git a/offload/plugins-nextgen/common/include/PluginInterface.h b/offload/plugins-nextgen/common/include/PluginInterface.h index 9de2698546e85..a5918718d09b3 100644 --- a/offload/plugins-nextgen/common/include/PluginInterface.h +++ b/offload/plugins-nextgen/common/include/PluginInterface.h @@ -1123,10 +1123,6 @@ struct GenericDeviceTy : public DeviceAllocatorTy { virtual Error dataMemcpyImpl(void *DstPtr, const void *SrcPtr, int64_t Size, AsyncInfoWrapperTy &AsyncInfoWrapper) = 0; - /// Instert a data fence between previous data operations and the following - /// operations if necessary for the device - virtual Error dataFence(__tgt_async_info *AsyncInfo) = 0; - /// Exchange data between devices (device to device transfer). Calling this /// function is only valid if GenericPlugin::isDataExchangable() passing the /// two devices returns true. @@ -1742,10 +1738,6 @@ struct GenericPluginTy { /// Notify t he runtime about a mapping that has been deleted. int32_t data_notify_unmapped(int32_t DeviceId, void *HstPtr); - /// Places a fence between previous data movements and following data - /// movements if necessary on the device - int32_t data_fence(int32_t DeviceId, __tgt_async_info *AsyncInfo); - /// Begin executing a kernel on the given device. int32_t launch_kernel(int32_t DeviceId, void *TgtEntryPtr, KernelLaunchArgsTy &LaunchArgs, diff --git a/offload/plugins-nextgen/common/src/PluginInterface.cpp b/offload/plugins-nextgen/common/src/PluginInterface.cpp index 2c3bbd7acbdce..9615e33cac306 100644 --- a/offload/plugins-nextgen/common/src/PluginInterface.cpp +++ b/offload/plugins-nextgen/common/src/PluginInterface.cpp @@ -1806,15 +1806,3 @@ int32_t GenericPluginTy::async_barrier(omp_interop_val_t *Interop) { } return OFFLOAD_SUCCESS; } - -int32_t GenericPluginTy::data_fence(int32_t DeviceId, - __tgt_async_info *AsyncInfo) { - auto Err = getDevice(DeviceId).dataFence(AsyncInfo); - if (Err) { - REPORT() << "Failure to place data fence on device " << DeviceId << ": " - << toString(std::move(Err)); - return OFFLOAD_FAIL; - } - - return OFFLOAD_SUCCESS; -} diff --git a/offload/plugins-nextgen/cuda/src/rtl.cpp b/offload/plugins-nextgen/cuda/src/rtl.cpp index 72e5dcf115fe9..e0135630f33cd 100644 --- a/offload/plugins-nextgen/cuda/src/rtl.cpp +++ b/offload/plugins-nextgen/cuda/src/rtl.cpp @@ -966,13 +966,6 @@ struct CUDADeviceTy : public GenericDeviceTy { return Plugin::success(); } - /// Insert a data fence between previous data operations and the following - /// operations. This is a no-op for CUDA devices as operations inserted into - /// a queue are in-order. - Error dataFence(__tgt_async_info *Async) override { - return Plugin::success(); - } - interop_spec_t selectInteropPreference(int32_t InteropType, int32_t NumPrefers, interop_spec_t *Prefers) override { diff --git a/offload/plugins-nextgen/host/src/rtl.cpp b/offload/plugins-nextgen/host/src/rtl.cpp index 55ada2f82c360..4a36120fc5ae4 100644 --- a/offload/plugins-nextgen/host/src/rtl.cpp +++ b/offload/plugins-nextgen/host/src/rtl.cpp @@ -293,13 +293,6 @@ struct GenELF64DeviceTy : public GenericDeviceTy { return Plugin::success(); } - /// Insert a data fence between previous data operations and the following - /// operations. This is a no-op for Host devices as operations inserted into - /// a queue are in-order. - Error dataFence(__tgt_async_info *Async) override { - return Plugin::success(); - } - Error dataFillImpl(void *TgtPtr, const void *PatternPtr, int64_t PatternSize, int64_t Size, AsyncInfoWrapperTy &AsyncInfoWrapper) override { diff --git a/offload/plugins-nextgen/level_zero/include/L0Device.h b/offload/plugins-nextgen/level_zero/include/L0Device.h index 332d16e739ce9..ffb4b09cb2b0a 100644 --- a/offload/plugins-nextgen/level_zero/include/L0Device.h +++ b/offload/plugins-nextgen/level_zero/include/L0Device.h @@ -534,7 +534,6 @@ class L0DeviceTy final : public GenericDeviceTy { } Expected<bool> isAccessiblePtrImpl(const void *Ptr, size_t Size) override; - Error dataFence(__tgt_async_info *Async) override; Error dataFillImpl(void *TgtPtr, const void *PatternPtr, int64_t PatternSize, int64_t Size, AsyncInfoWrapperTy &AsyncInfoWrapper) override; diff --git a/offload/plugins-nextgen/level_zero/src/L0Device.cpp b/offload/plugins-nextgen/level_zero/src/L0Device.cpp index f6900731ce80f..d389f25e70009 100644 --- a/offload/plugins-nextgen/level_zero/src/L0Device.cpp +++ b/offload/plugins-nextgen/level_zero/src/L0Device.cpp @@ -754,14 +754,6 @@ L0DeviceTy::createImmCmdList(uint32_t Ordinal, uint32_t Index, return CmdList; } -Error L0DeviceTy::dataFence(__tgt_async_info *Async) { - auto QueueOrErr = getOrCreateQueue(Async); - if (!QueueOrErr) - return QueueOrErr.takeError(); - L0QueueTy *Queue = *QueueOrErr; - return Queue->dataFence(); -} - Expected<bool> L0DeviceTy::isAccessiblePtrImpl(const void *Ptr, size_t Size) { if (!Ptr || Size == 0) return Plugin::error(ErrorCode::INVALID_ARGUMENT, _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
