From 416e5ed82f64ca95dcd12ed4a5800fe9fbac621b Mon Sep 17 00:00:00 2001 From: Ross Brunton Date: Thu, 12 Jun 2025 15:07:25 +0100 Subject: [PATCH 1/2] [Offload] Add `ol_dimensions_t` This is a three element x, y, z size_t vector that can be used any place where a 3D vector is required. This ensures that all vectors across liboffload are the same and don't require any resizing/reordering dances. --- offload/liboffload/API/Common.td | 10 ++++++++++ offload/liboffload/API/Kernel.td | 8 ++------ offload/liboffload/src/OffloadImpl.cpp | 12 ++++++------ .../unittests/OffloadAPI/kernel/olLaunchKernel.cpp | 13 ++++--------- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/offload/liboffload/API/Common.td b/offload/liboffload/API/Common.td index 7674da0438c29..79356029972fa 100644 --- a/offload/liboffload/API/Common.td +++ b/offload/liboffload/API/Common.td @@ -148,6 +148,16 @@ def : Struct { ]; } +def : Struct { + let name = "ol_dimensions_t"; + let desc = "A three element vector"; + let members = [ + StructMember<"size_t", "x", "X">, + StructMember<"size_t", "y", "Y">, + StructMember<"size_t", "z", "Z">, + ]; +} + def : Function { let name = "olInit"; let desc = "Perform initialization of the Offload library and plugins"; diff --git a/offload/liboffload/API/Kernel.td b/offload/liboffload/API/Kernel.td index 45e3d8112791c..0913a036fa04f 100644 --- a/offload/liboffload/API/Kernel.td +++ b/offload/liboffload/API/Kernel.td @@ -29,12 +29,8 @@ def : Struct { let desc = "Size-related arguments for a kernel launch."; let members = [ StructMember<"size_t", "Dimensions", "Number of work dimensions">, - StructMember<"size_t", "NumGroupsX", "Number of work groups on the X dimension">, - StructMember<"size_t", "NumGroupsY", "Number of work groups on the Y dimension">, - StructMember<"size_t", "NumGroupsZ", "Number of work groups on the Z dimension">, - StructMember<"size_t", "GroupSizeX", "Size of a work group on the X dimension.">, - StructMember<"size_t", "GroupSizeY", "Size of a work group on the Y dimension.">, - StructMember<"size_t", "GroupSizeZ", "Size of a work group on the Z dimension.">, + StructMember<"struct ol_dimensions_t", "NumGroups", "Number of work groups in each dimension">, + StructMember<"struct ol_dimensions_t", "GroupSize", "Size of a work group in each dimension">, StructMember<"size_t", "DynSharedMemory", "Size of dynamic shared memory in bytes."> ]; } diff --git a/offload/liboffload/src/OffloadImpl.cpp b/offload/liboffload/src/OffloadImpl.cpp index d2b331905ab77..0a784cddeaecb 100644 --- a/offload/liboffload/src/OffloadImpl.cpp +++ b/offload/liboffload/src/OffloadImpl.cpp @@ -499,12 +499,12 @@ Error olLaunchKernel_impl(ol_queue_handle_t Queue, ol_device_handle_t Device, auto *QueueImpl = Queue ? Queue->AsyncInfo : nullptr; AsyncInfoWrapperTy AsyncInfoWrapper(*DeviceImpl, QueueImpl); KernelArgsTy LaunchArgs{}; - LaunchArgs.NumTeams[0] = LaunchSizeArgs->NumGroupsX; - LaunchArgs.NumTeams[1] = LaunchSizeArgs->NumGroupsY; - LaunchArgs.NumTeams[2] = LaunchSizeArgs->NumGroupsZ; - LaunchArgs.ThreadLimit[0] = LaunchSizeArgs->GroupSizeX; - LaunchArgs.ThreadLimit[1] = LaunchSizeArgs->GroupSizeY; - LaunchArgs.ThreadLimit[2] = LaunchSizeArgs->GroupSizeZ; + LaunchArgs.NumTeams[0] = LaunchSizeArgs->NumGroups.x; + LaunchArgs.NumTeams[1] = LaunchSizeArgs->NumGroups.y; + LaunchArgs.NumTeams[2] = LaunchSizeArgs->NumGroups.z; + LaunchArgs.ThreadLimit[0] = LaunchSizeArgs->GroupSize.x; + LaunchArgs.ThreadLimit[1] = LaunchSizeArgs->GroupSize.y; + LaunchArgs.ThreadLimit[2] = LaunchSizeArgs->GroupSize.z; LaunchArgs.DynCGroupMem = LaunchSizeArgs->DynSharedMemory; KernelLaunchParamsTy Params; diff --git a/offload/unittests/OffloadAPI/kernel/olLaunchKernel.cpp b/offload/unittests/OffloadAPI/kernel/olLaunchKernel.cpp index d466799c1acaa..157f33a363700 100644 --- a/offload/unittests/OffloadAPI/kernel/olLaunchKernel.cpp +++ b/offload/unittests/OffloadAPI/kernel/olLaunchKernel.cpp @@ -19,13 +19,8 @@ struct LaunchKernelTestBase : OffloadQueueTest { DeviceBin->getBufferSize(), &Program)); ASSERT_SUCCESS(olGetKernel(Program, kernel, &Kernel)); LaunchArgs.Dimensions = 1; - LaunchArgs.GroupSizeX = 64; - LaunchArgs.GroupSizeY = 1; - LaunchArgs.GroupSizeZ = 1; - - LaunchArgs.NumGroupsX = 1; - LaunchArgs.NumGroupsY = 1; - LaunchArgs.NumGroupsZ = 1; + LaunchArgs.GroupSize = {64, 1, 1}; + LaunchArgs.NumGroups = {1, 1, 1}; LaunchArgs.DynSharedMemory = 0; } @@ -60,7 +55,7 @@ OFFLOAD_TESTS_INSTANTIATE_DEVICE_FIXTURE(olLaunchKernelNoArgsTest); TEST_P(olLaunchKernelTest, Success) { void *Mem; ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_MANAGED, - LaunchArgs.GroupSizeX * sizeof(uint32_t), &Mem)); + LaunchArgs.GroupSize.x * sizeof(uint32_t), &Mem)); struct { void *Mem; } Args{Mem}; @@ -88,7 +83,7 @@ TEST_P(olLaunchKernelNoArgsTest, Success) { TEST_P(olLaunchKernelTest, SuccessSynchronous) { void *Mem; ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_MANAGED, - LaunchArgs.GroupSizeX * sizeof(uint32_t), &Mem)); + LaunchArgs.GroupSize.x * sizeof(uint32_t), &Mem)); struct { void *Mem; From f27f64821eebb0901c9a92875dfd514dddf8705f Mon Sep 17 00:00:00 2001 From: Ross Brunton Date: Thu, 12 Jun 2025 15:29:23 +0100 Subject: [PATCH 2/2] Use uint32_t rather than size_t for ranges --- offload/liboffload/API/Common.td | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/offload/liboffload/API/Common.td b/offload/liboffload/API/Common.td index 79356029972fa..8a2ecd6c6e8f4 100644 --- a/offload/liboffload/API/Common.td +++ b/offload/liboffload/API/Common.td @@ -152,9 +152,9 @@ def : Struct { let name = "ol_dimensions_t"; let desc = "A three element vector"; let members = [ - StructMember<"size_t", "x", "X">, - StructMember<"size_t", "y", "Y">, - StructMember<"size_t", "z", "Z">, + StructMember<"uint32_t", "x", "X">, + StructMember<"uint32_t", "y", "Y">, + StructMember<"uint32_t", "z", "Z">, ]; }