From 051aa2a575c4b4dc7bbe2a5f138dc4be7bf17707 Mon Sep 17 00:00:00 2001 From: Victor Lomuller Date: Mon, 29 Jul 2024 16:43:14 +0100 Subject: [PATCH 1/5] [SYCL][AMD] Propagate metadata in createURPRogram SYCL properties weren't converted when calling creatreURProgram, leading to issue in finalization during KernelFusion for AMD. --- sycl/source/detail/program_manager/program_manager.cpp | 8 ++++++-- sycl/test-e2e/KernelFusion/lit.local.cfg | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index ab0eb8a00f13a..66a00c19139d8 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -216,7 +216,11 @@ ProgramManager::createURProgram(const RTDeviceBinaryImage &Img, "SPIR-V online compilation is not supported in this context"); // Get program metadata from properties - auto ProgMetadata = Img.getProgramMetadataUR(); + std::vector ProgMetadataVector; + for (const auto &Prop : Img.getProgramMetadata()) { + ProgMetadataVector.push_back( + ur::mapDeviceBinaryPropertyToProgramMetadata(Prop)); + } // Load the image const ContextImplPtr Ctx = getSyclObjImpl(Context); @@ -224,7 +228,7 @@ ProgramManager::createURProgram(const RTDeviceBinaryImage &Img, Format == SYCL_DEVICE_BINARY_TYPE_SPIRV ? createSpirvProgram(Ctx, RawImg.BinaryStart, ImgSize) : createBinaryProgram(Ctx, Device, RawImg.BinaryStart, ImgSize, - ProgMetadata); + ProgMetadataVector); { std::lock_guard Lock(MNativeProgramsMutex); diff --git a/sycl/test-e2e/KernelFusion/lit.local.cfg b/sycl/test-e2e/KernelFusion/lit.local.cfg index cc77315a316ef..f6ea07e50493f 100644 --- a/sycl/test-e2e/KernelFusion/lit.local.cfg +++ b/sycl/test-e2e/KernelFusion/lit.local.cfg @@ -2,7 +2,7 @@ import platform config.required_features += ['fusion'] # TODO: Reenable hip, see https://github.com/intel/llvm/issues/14598 -config.unsupported_features += ['accelerator', 'hip'] +config.unsupported_features += ['accelerator'] # TODO: enable on Windows once kernel fusion is supported on Windows. if platform.system() != "Linux": From 8cea64aca465e7ba12b0b3b1ac3c03ae9f7b391e Mon Sep 17 00:00:00 2001 From: Victor Lomuller Date: Mon, 29 Jul 2024 17:02:29 +0100 Subject: [PATCH 2/5] ensure getProgramMetadataUR is filled --- sycl/source/detail/device_binary_image.cpp | 5 +++++ .../detail/program_manager/program_manager.cpp | 16 +++++----------- sycl/test-e2e/KernelFusion/lit.local.cfg | 1 - 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/sycl/source/detail/device_binary_image.cpp b/sycl/source/detail/device_binary_image.cpp index a1d89ed845da2..beb9bae0dd0f1 100644 --- a/sycl/source/detail/device_binary_image.cpp +++ b/sycl/source/detail/device_binary_image.cpp @@ -178,6 +178,11 @@ void RTDeviceBinaryImage::init(sycl_device_binary Bin) { KernelParamOptInfo.init(Bin, __SYCL_PROPERTY_SET_KERNEL_PARAM_OPT_INFO); AssertUsed.init(Bin, __SYCL_PROPERTY_SET_SYCL_ASSERT_USED); ProgramMetadata.init(Bin, __SYCL_PROPERTY_SET_PROGRAM_METADATA); + // Convert ProgramMetadata into the UR format + for (const auto &Prop : ProgramMetadata) { + ProgramMetadataUR.push_back( + ur::mapDeviceBinaryPropertyToProgramMetadata(Prop)); + } ExportedSymbols.init(Bin, __SYCL_PROPERTY_SET_SYCL_EXPORTED_SYMBOLS); ImportedSymbols.init(Bin, __SYCL_PROPERTY_SET_SYCL_IMPORTED_SYMBOLS); DeviceGlobals.init(Bin, __SYCL_PROPERTY_SET_SYCL_DEVICE_GLOBALS); diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index 66a00c19139d8..92c5e9ed8f132 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -216,11 +216,7 @@ ProgramManager::createURProgram(const RTDeviceBinaryImage &Img, "SPIR-V online compilation is not supported in this context"); // Get program metadata from properties - std::vector ProgMetadataVector; - for (const auto &Prop : Img.getProgramMetadata()) { - ProgMetadataVector.push_back( - ur::mapDeviceBinaryPropertyToProgramMetadata(Prop)); - } + auto ProgMetadata = Img.getProgramMetadataUR(); // Load the image const ContextImplPtr Ctx = getSyclObjImpl(Context); @@ -228,7 +224,7 @@ ProgramManager::createURProgram(const RTDeviceBinaryImage &Img, Format == SYCL_DEVICE_BINARY_TYPE_SPIRV ? createSpirvProgram(Ctx, RawImg.BinaryStart, ImgSize) : createBinaryProgram(Ctx, Device, RawImg.BinaryStart, ImgSize, - ProgMetadataVector); + ProgMetadata); { std::lock_guard Lock(MNativeProgramsMutex); @@ -504,11 +500,9 @@ std::pair ProgramManager::getOrCreateURProgram( // Get program metadata from properties std::vector ProgMetadataVector; for (const RTDeviceBinaryImage *Img : AllImages) { - auto ProgMetadata = Img->getProgramMetadata(); - for (const auto &Prop : ProgMetadata) { - ProgMetadataVector.push_back( - ur::mapDeviceBinaryPropertyToProgramMetadata(Prop)); - } + auto &ImgProgMetadata = Img->getProgramMetadataUR(); + ProgMetadataVector.insert(ProgMetadataVector.end(), + ImgProgMetadata.begin(), ImgProgMetadata.end()); } // TODO: Build for multiple devices once supported by program manager NativePrg = createBinaryProgram(getSyclObjImpl(Context), Device, diff --git a/sycl/test-e2e/KernelFusion/lit.local.cfg b/sycl/test-e2e/KernelFusion/lit.local.cfg index f6ea07e50493f..1d0db3020f754 100644 --- a/sycl/test-e2e/KernelFusion/lit.local.cfg +++ b/sycl/test-e2e/KernelFusion/lit.local.cfg @@ -1,7 +1,6 @@ import platform config.required_features += ['fusion'] -# TODO: Reenable hip, see https://github.com/intel/llvm/issues/14598 config.unsupported_features += ['accelerator'] # TODO: enable on Windows once kernel fusion is supported on Windows. From df109cc6f198e47ca0808ebf112ab732d588de9f Mon Sep 17 00:00:00 2001 From: Victor Lomuller Date: Tue, 30 Jul 2024 15:57:11 +0100 Subject: [PATCH 3/5] Re-enable, Fixes #14598 --- sycl/test-e2e/Basic/reqd_work_group_size.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/sycl/test-e2e/Basic/reqd_work_group_size.cpp b/sycl/test-e2e/Basic/reqd_work_group_size.cpp index f52ab51a4f8d4..d3fbe1621c757 100644 --- a/sycl/test-e2e/Basic/reqd_work_group_size.cpp +++ b/sycl/test-e2e/Basic/reqd_work_group_size.cpp @@ -1,9 +1,6 @@ // RUN: %{build} -o %t.out // RUN: %{run} %t.out -// TODO: Reenable, see https://github.com/intel/llvm/issues/14598 -// UNSUPPORTED: linux, windows - #include #include From e9665d8af3d549513ae985cdd3668b8a21327743 Mon Sep 17 00:00:00 2001 From: Victor Lomuller Date: Tue, 30 Jul 2024 16:13:17 +0100 Subject: [PATCH 4/5] re-enable diamond_shape --- sycl/test-e2e/NewOffloadDriver/diamond_shape.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/sycl/test-e2e/NewOffloadDriver/diamond_shape.cpp b/sycl/test-e2e/NewOffloadDriver/diamond_shape.cpp index af760cb13c605..d3fb670b6bb75 100644 --- a/sycl/test-e2e/NewOffloadDriver/diamond_shape.cpp +++ b/sycl/test-e2e/NewOffloadDriver/diamond_shape.cpp @@ -1,6 +1,4 @@ // REQUIRES: fusion -// TODO: Reenable, see https://github.com/intel/llvm/issues/14598 -// UNSUPPORTED: hip // RUN: %{build} %{embed-ir} -O2 --offload-new-driver -o %t.out // RUN: %{run} %t.out From d713bbb922f20c6a6f7508e6bf2ed6eb59afa6f3 Mon Sep 17 00:00:00 2001 From: Victor Lomuller Date: Wed, 31 Jul 2024 13:34:42 +0100 Subject: [PATCH 5/5] re-enable more tests --- sycl/test-e2e/DeviceGlobal/device_global_arrow.cpp | 3 +-- sycl/test-e2e/DeviceGlobal/device_global_device_only.cpp | 3 +-- .../DeviceGlobal/device_global_operator_passthrough.cpp | 3 +-- sycl/test-e2e/DeviceGlobal/device_global_subscript.cpp | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/sycl/test-e2e/DeviceGlobal/device_global_arrow.cpp b/sycl/test-e2e/DeviceGlobal/device_global_arrow.cpp index 24b0437f2a35c..ffcadf8667bda 100644 --- a/sycl/test-e2e/DeviceGlobal/device_global_arrow.cpp +++ b/sycl/test-e2e/DeviceGlobal/device_global_arrow.cpp @@ -3,8 +3,7 @@ // // The OpenCL GPU backends do not currently support device_global backend // calls. -// TODO: Reenable linux/windows, see https://github.com/intel/llvm/issues/14598 -// UNSUPPORTED: opencl && gpu, linux, windows +// UNSUPPORTED: opencl && gpu // // Tests operator-> on device_global. diff --git a/sycl/test-e2e/DeviceGlobal/device_global_device_only.cpp b/sycl/test-e2e/DeviceGlobal/device_global_device_only.cpp index 07ea4f0ec94b4..ac2894c13c855 100644 --- a/sycl/test-e2e/DeviceGlobal/device_global_device_only.cpp +++ b/sycl/test-e2e/DeviceGlobal/device_global_device_only.cpp @@ -3,8 +3,7 @@ // // The OpenCL GPU backends do not currently support device_global backend // calls. -// TODO: Reenable linux/windows, see https://github.com/intel/llvm/issues/14598 -// UNSUPPORTED: opencl && gpu, linux, windows +// UNSUPPORTED: opencl && gpu // // Tests basic device_global access through device kernels. diff --git a/sycl/test-e2e/DeviceGlobal/device_global_operator_passthrough.cpp b/sycl/test-e2e/DeviceGlobal/device_global_operator_passthrough.cpp index c98a22b851df6..b687bb4c4365d 100644 --- a/sycl/test-e2e/DeviceGlobal/device_global_operator_passthrough.cpp +++ b/sycl/test-e2e/DeviceGlobal/device_global_operator_passthrough.cpp @@ -3,8 +3,7 @@ // // The OpenCL GPU backends do not currently support device_global backend // calls. -// TODO: Reenable linux/windows, see https://github.com/intel/llvm/issues/14598 -// UNSUPPORTED: opencl && gpu, linux, windows +// UNSUPPORTED: opencl && gpu // // Tests the passthrough of operators on device_global. diff --git a/sycl/test-e2e/DeviceGlobal/device_global_subscript.cpp b/sycl/test-e2e/DeviceGlobal/device_global_subscript.cpp index e519db2894993..cec40fafd61f3 100644 --- a/sycl/test-e2e/DeviceGlobal/device_global_subscript.cpp +++ b/sycl/test-e2e/DeviceGlobal/device_global_subscript.cpp @@ -3,8 +3,7 @@ // // The OpenCL GPU backends do not currently support device_global backend // calls. -// TODO: Reenable linux/windows, see https://github.com/intel/llvm/issues/14598 -// UNSUPPORTED: opencl && gpu, linux, windows +// UNSUPPORTED: opencl && gpu // // Tests operator[] on device_global.