From 1d9bf7a5bbd6b7f616544a3d2e9094fe29a2ea08 Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Tue, 18 Jun 2024 02:33:06 -0700 Subject: [PATCH 1/6] [SYCL] Add basic tests for virtual functions --- .../2/1/1/missing-overrides.cpp | 144 ++++++++++++++++++ .../2/1/1/more-complex-hierarchy.cpp | 112 ++++++++++++++ .../2/1/1/simple-hierarchy.cpp | 97 ++++++++++++ sycl/test-e2e/VirtualFunctions/README.md | 7 + 4 files changed, 360 insertions(+) create mode 100644 sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp create mode 100644 sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp create mode 100644 sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp create mode 100644 sycl/test-e2e/VirtualFunctions/README.md diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp new file mode 100644 index 0000000000000..c719e4222f83c --- /dev/null +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp @@ -0,0 +1,144 @@ +// UNSUPPORTED: cuda, hip, acc +// FIXME: replace unsupported with an aspect check once we have it +// +// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions +// RUN: %{run} %t.out + +#include + +#include + +namespace oneapi = sycl::ext::oneapi::experimental; + +class Base { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + virtual void increment(int *) { /* do nothhing */ + } + + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + virtual void multiply(int *) { /* do nothhing */ + } + + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + virtual void substract(int *) { /* do nothhing */ + } +}; + +class IncrementBy1 : public Base { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 1; } +}; + +class IncrementBy1AndSubstractBy2 : public IncrementBy1 { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void substract(int *Data) override { *Data -= 2; } +}; + +class MultiplyBy2 : public Base { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void multiply(int *Data) override { *Data *= 2; } +}; + +class MultiplyBy2AndIncrementBy8 : public MultiplyBy2 { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 8; } +}; + +class SubstractBy4 : public Base { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void substract(int *Data) override { *Data -= 4; } +}; + +class SubstractBy4AndMultiplyBy4 : public SubstractBy4 { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void multiply(int *Data) override { *Data *= 4; } +}; + +Base *constructAnObject(char *Storage, int Index) { + switch (Index) { + case 0: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy1; + return Ret; + } + case 1: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy1AndSubstractBy2; + return Ret; + } + case 2: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) MultiplyBy2; + return Ret; + } + case 3: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) MultiplyBy2AndIncrementBy8; + return Ret; + } + case 4: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) SubstractBy4; + return Ret; + } + case 5: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) SubstractBy4AndMultiplyBy4; + return Ret; + } + + default: + return nullptr; + } +} + +void applyOp(int *DataPtr, Base *ObjPtr) { + ObjPtr->increment(DataPtr); + ObjPtr->substract(DataPtr); + ObjPtr->multiply(DataPtr); +} + +int main() { + constexpr size_t Size = + std::max({sizeof(IncrementBy1), sizeof(IncrementBy1AndSubstractBy2), + sizeof(MultiplyBy2), sizeof(MultiplyBy2AndIncrementBy8), + sizeof(SubstractBy4), sizeof(SubstractBy4AndMultiplyBy4)}); + + sycl::buffer ObjStorage(sycl::range{Size}); + char HostStorage[Size]; + sycl::queue q; + + constexpr oneapi::properties props{oneapi::calls_indirectly<>}; + for (int TestCase = 0; TestCase < 6; ++TestCase) { + int HostData = 42; + int Data = HostData; + sycl::buffer DataStorage(&Data, sycl::range{1}); + + q.submit([&](sycl::handler &CGH) { + sycl::accessor StorageAcc(ObjStorage, CGH, sycl::write_only); + sycl::accessor DataAcc(DataStorage, CGH, sycl::write_only); + CGH.single_task(props, [=]() { + Base *Ptr = constructAnObject( + StorageAcc.get_multi_ptr().get(), + TestCase); + applyOp(DataAcc.get_multi_ptr().get(), + Ptr); + }); + }); + + Base *Ptr = constructAnObject(HostStorage, TestCase); + applyOp(&HostData, Ptr); + + sycl::host_accessor HostAcc(DataStorage); + assert(HostAcc[0] == HostData); + } + + return 0; +} diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp new file mode 100644 index 0000000000000..a52d084c21f22 --- /dev/null +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp @@ -0,0 +1,112 @@ +// UNSUPPORTED: cuda, hip, acc +// FIXME: replace unsupported with an aspect check once we have it +// +// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions +// RUN: %{run} %t.out + +#include + +#include + +namespace oneapi = sycl::ext::oneapi::experimental; + +class AbstractOp { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + virtual void applyOp(int *) = 0; +}; + +class IncrementOp : public AbstractOp { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void applyOp(int *Data) final override { increment(Data); } + + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + virtual void increment(int *) = 0; +}; + +class IncrementBy1 : public IncrementOp { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 1; } +}; + +class IncrementBy2 : public IncrementOp { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 2; } +}; + +class IncrementBy4 : public IncrementOp { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 4; } +}; + +class IncrementBy8 : public IncrementOp { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 8; } +}; + +AbstractOp *constructAnObject(char *Storage, int Index) { + switch (Index) { + case 0: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy1; + return Ret; + } + case 1: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy2; + return Ret; + } + case 2: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy4; + return Ret; + } + case 3: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy8; + return Ret; + } + + default: + return nullptr; + } +} + +void applyOp(int *Data, AbstractOp *Obj) { Obj->applyOp(Data); } + +int main() { + constexpr size_t Size = + std::max({sizeof(IncrementBy1), sizeof(IncrementBy2), + sizeof(IncrementBy4), sizeof(IncrementBy8)}); + + sycl::buffer ObjStorage(sycl::range{Size}); + char HostStorage[Size]; + sycl::queue q; + + constexpr oneapi::properties props{oneapi::calls_indirectly<>}; + for (int TestCase = 0; TestCase < 4; ++TestCase) { + int HostData = 42; + int Data = HostData; + sycl::buffer DataStorage(&Data, sycl::range{1}); + + q.submit([&](sycl::handler &CGH) { + sycl::accessor StorageAcc(ObjStorage, CGH, sycl::write_only); + sycl::accessor DataAcc(DataStorage, CGH, sycl::write_only); + CGH.single_task(props, [=]() { + AbstractOp *Ptr = constructAnObject( + StorageAcc.get_multi_ptr().get(), + TestCase); + applyOp(DataAcc.get_multi_ptr().get(), + Ptr); + }); + }); + + AbstractOp *Ptr = constructAnObject(HostStorage, TestCase); + Ptr->applyOp(&HostData); + + sycl::host_accessor HostAcc(DataStorage); + assert(HostAcc[0] == HostData); + } + + return 0; +} diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp new file mode 100644 index 0000000000000..84a296cbedc8b --- /dev/null +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp @@ -0,0 +1,97 @@ +// UNSUPPORTED: cuda, hip, acc +// FIXME: replace unsupported with an aspect check once we have it +// +// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions +// RUN: %{run} %t.out + +#include + +#include + +namespace oneapi = sycl::ext::oneapi::experimental; + +class BaseIncrement { +public: + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + virtual void increment(int *Data) { *Data += 1; } +}; + +class IncrementBy2 : public BaseIncrement { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 2; } +}; + +class IncrementBy4 : public BaseIncrement { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 4; } +}; + +class IncrementBy8 : public BaseIncrement { + SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) + void increment(int *Data) override { *Data += 8; } +}; + +BaseIncrement *constructAnObject(char *Storage, int Index) { + switch (Index) { + case 0: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) BaseIncrement; + return Ret; + } + case 1: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy2; + return Ret; + } + case 2: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy4; + return Ret; + } + case 3: { + auto *Ret = reinterpret_cast(Storage); + new (Storage) IncrementBy8; + return Ret; + } + + default: + return nullptr; + } +} + +int main() { + constexpr size_t Size = + std::max({sizeof(BaseIncrement), sizeof(IncrementBy2), + sizeof(IncrementBy4), sizeof(IncrementBy8)}); + + sycl::buffer ObjStorage(sycl::range{Size}); + char HostStorage[Size]; + sycl::queue q; + + constexpr oneapi::properties props{oneapi::calls_indirectly<>}; + for (int TestCase = 0; TestCase < 4; ++TestCase) { + int HostData = 42; + int Data = HostData; + sycl::buffer DataStorage(&Data, sycl::range{1}); + + q.submit([&](sycl::handler &CGH) { + sycl::accessor StorageAcc(ObjStorage, CGH, sycl::write_only); + sycl::accessor DataAcc(DataStorage, CGH, sycl::write_only); + CGH.single_task(props, [=]() { + BaseIncrement *Ptr = constructAnObject( + StorageAcc.get_multi_ptr().get(), + TestCase); + Ptr->increment( + DataAcc.get_multi_ptr().get()); + }); + }); + + BaseIncrement *Ptr = constructAnObject(HostStorage, TestCase); + Ptr->increment(&HostData); + + sycl::host_accessor HostAcc(DataStorage); + assert(HostAcc[0] == HostData); + } + + return 0; +} diff --git a/sycl/test-e2e/VirtualFunctions/README.md b/sycl/test-e2e/VirtualFunctions/README.md new file mode 100644 index 0000000000000..8eef65107eec7 --- /dev/null +++ b/sycl/test-e2e/VirtualFunctions/README.md @@ -0,0 +1,7 @@ +# E2E tests for `sycl_ext_oneapi_virtual_functions` extension + +Note about naming convention and files organization for this folder: the tests, +files and directories are named and organized in a way that resembles their +description in the corresponding test plan document: link to be inserted here +later, but for now look into +[intel/llvm#10540](https://github.com/intel/llvm/pull/10540) PR. From b6d653b752e86f5acf27e6f631585b3e741d466d Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Thu, 20 Jun 2024 06:15:30 -0700 Subject: [PATCH 2/6] Switch to using core.hpp instead of sycl.hpp --- sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp | 2 +- sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp | 2 +- sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp index c719e4222f83c..e0f105ccfb5bd 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp @@ -4,7 +4,7 @@ // RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions // RUN: %{run} %t.out -#include +#include #include diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp index a52d084c21f22..86ecb27de3cd5 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp @@ -4,7 +4,7 @@ // RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions // RUN: %{run} %t.out -#include +#include #include diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp index 84a296cbedc8b..5768e7d41e365 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp @@ -4,7 +4,7 @@ // RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions // RUN: %{run} %t.out -#include +#include #include From 430a8fec9d1d7c4587d698aaeb3008911b5cd568 Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Thu, 20 Jun 2024 06:51:00 -0700 Subject: [PATCH 3/6] Apply clang-format --- .../VirtualFunctions/2/1/1/missing-overrides.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp index e0f105ccfb5bd..08126ecb58828 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp @@ -13,16 +13,13 @@ namespace oneapi = sycl::ext::oneapi::experimental; class Base { public: SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) - virtual void increment(int *) { /* do nothhing */ - } + virtual void increment(int *) { /* do nothhing */ } SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) - virtual void multiply(int *) { /* do nothhing */ - } + virtual void multiply(int *) { /* do nothhing */ } SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) - virtual void substract(int *) { /* do nothhing */ - } + virtual void substract(int *) { /* do nothhing */ } }; class IncrementBy1 : public Base { From 4e29ba59972c27a7c4638be9c26734a48a7f6897 Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Thu, 20 Jun 2024 08:00:34 -0700 Subject: [PATCH 4/6] One more attempt to use the right header in E2E tests --- sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp | 2 +- sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp | 2 +- sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp index 08126ecb58828..305fb80076373 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp @@ -4,7 +4,7 @@ // RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions // RUN: %{run} %t.out -#include +#include #include diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp index 86ecb27de3cd5..3f2fc6f337fb5 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp @@ -4,7 +4,7 @@ // RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions // RUN: %{run} %t.out -#include +#include #include diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp index 5768e7d41e365..f263cec37dba2 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp @@ -4,7 +4,7 @@ // RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions // RUN: %{run} %t.out -#include +#include #include From 04dda014340218f9c2696c404029886309052442 Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Thu, 11 Jul 2024 03:38:38 -0700 Subject: [PATCH 5/6] Introduce more helper structures Main motivation is to make sure that storage for objects is properly aligned so there are no UBs when using placement new. It also allowed to simplify tests source code by outlining some common/similar object construction part into a helper. --- .../2/1/1/missing-overrides.cpp | 63 ++++--------------- .../2/1/1/more-complex-hierarchy.cpp | 53 ++++------------ .../2/1/1/simple-hierarchy.cpp | 53 ++++------------ sycl/test-e2e/VirtualFunctions/helpers.hpp | 47 ++++++++++++++ sycl/test-e2e/VirtualFunctions/lit.local.cfg | 6 ++ 5 files changed, 88 insertions(+), 134 deletions(-) create mode 100644 sycl/test-e2e/VirtualFunctions/helpers.hpp create mode 100644 sycl/test-e2e/VirtualFunctions/lit.local.cfg diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp index 305fb80076373..6ae83310333d5 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp @@ -1,12 +1,12 @@ // UNSUPPORTED: cuda, hip, acc // FIXME: replace unsupported with an aspect check once we have it // -// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions +// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions %helper-includes // RUN: %{run} %t.out #include -#include +#include "helpers.hpp" namespace oneapi = sycl::ext::oneapi::experimental; @@ -58,44 +58,6 @@ class SubstractBy4AndMultiplyBy4 : public SubstractBy4 { void multiply(int *Data) override { *Data *= 4; } }; -Base *constructAnObject(char *Storage, int Index) { - switch (Index) { - case 0: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy1; - return Ret; - } - case 1: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy1AndSubstractBy2; - return Ret; - } - case 2: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) MultiplyBy2; - return Ret; - } - case 3: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) MultiplyBy2AndIncrementBy8; - return Ret; - } - case 4: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) SubstractBy4; - return Ret; - } - case 5: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) SubstractBy4AndMultiplyBy4; - return Ret; - } - - default: - return nullptr; - } -} - void applyOp(int *DataPtr, Base *ObjPtr) { ObjPtr->increment(DataPtr); ObjPtr->substract(DataPtr); @@ -103,34 +65,31 @@ void applyOp(int *DataPtr, Base *ObjPtr) { } int main() { - constexpr size_t Size = - std::max({sizeof(IncrementBy1), sizeof(IncrementBy1AndSubstractBy2), - sizeof(MultiplyBy2), sizeof(MultiplyBy2AndIncrementBy8), - sizeof(SubstractBy4), sizeof(SubstractBy4AndMultiplyBy4)}); + using storage_t = obj_storage_t; + storage_t HostStorage; + sycl::buffer DeviceStorage(sycl::range{1}); - sycl::buffer ObjStorage(sycl::range{Size}); - char HostStorage[Size]; sycl::queue q; constexpr oneapi::properties props{oneapi::calls_indirectly<>}; - for (int TestCase = 0; TestCase < 6; ++TestCase) { + for (unsigned TestCase = 0; TestCase < 6; ++TestCase) { int HostData = 42; int Data = HostData; sycl::buffer DataStorage(&Data, sycl::range{1}); q.submit([&](sycl::handler &CGH) { - sycl::accessor StorageAcc(ObjStorage, CGH, sycl::write_only); + sycl::accessor StorageAcc(DeviceStorage, CGH, sycl::write_only); sycl::accessor DataAcc(DataStorage, CGH, sycl::write_only); CGH.single_task(props, [=]() { - Base *Ptr = constructAnObject( - StorageAcc.get_multi_ptr().get(), - TestCase); + auto *Ptr = StorageAcc[0].construct(TestCase); applyOp(DataAcc.get_multi_ptr().get(), Ptr); }); }); - Base *Ptr = constructAnObject(HostStorage, TestCase); + Base *Ptr = HostStorage.construct(TestCase); applyOp(&HostData, Ptr); sycl::host_accessor HostAcc(DataStorage); diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp index 3f2fc6f337fb5..e8b86bae7481a 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp @@ -1,12 +1,12 @@ // UNSUPPORTED: cuda, hip, acc // FIXME: replace unsupported with an aspect check once we have it // -// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions +// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions %helper-includes // RUN: %{run} %t.out #include -#include +#include "helpers.hpp" namespace oneapi = sycl::ext::oneapi::experimental; @@ -44,64 +44,35 @@ class IncrementBy8 : public IncrementOp { void increment(int *Data) override { *Data += 8; } }; -AbstractOp *constructAnObject(char *Storage, int Index) { - switch (Index) { - case 0: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy1; - return Ret; - } - case 1: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy2; - return Ret; - } - case 2: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy4; - return Ret; - } - case 3: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy8; - return Ret; - } - - default: - return nullptr; - } -} - void applyOp(int *Data, AbstractOp *Obj) { Obj->applyOp(Data); } int main() { - constexpr size_t Size = - std::max({sizeof(IncrementBy1), sizeof(IncrementBy2), - sizeof(IncrementBy4), sizeof(IncrementBy8)}); + using storage_t = + obj_storage_t; + + storage_t HostStorage; + sycl::buffer DeviceStorage(sycl::range{1}); - sycl::buffer ObjStorage(sycl::range{Size}); - char HostStorage[Size]; sycl::queue q; constexpr oneapi::properties props{oneapi::calls_indirectly<>}; - for (int TestCase = 0; TestCase < 4; ++TestCase) { + for (unsigned TestCase = 0; TestCase < 4; ++TestCase) { int HostData = 42; int Data = HostData; sycl::buffer DataStorage(&Data, sycl::range{1}); q.submit([&](sycl::handler &CGH) { - sycl::accessor StorageAcc(ObjStorage, CGH, sycl::write_only); + sycl::accessor StorageAcc(DeviceStorage, CGH, sycl::write_only); sycl::accessor DataAcc(DataStorage, CGH, sycl::write_only); CGH.single_task(props, [=]() { - AbstractOp *Ptr = constructAnObject( - StorageAcc.get_multi_ptr().get(), - TestCase); + auto *Ptr = + StorageAcc[0].construct(TestCase); applyOp(DataAcc.get_multi_ptr().get(), Ptr); }); }); - AbstractOp *Ptr = constructAnObject(HostStorage, TestCase); + auto *Ptr = HostStorage.construct(TestCase); Ptr->applyOp(&HostData); sycl::host_accessor HostAcc(DataStorage); diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp index f263cec37dba2..98ca012faa044 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp @@ -1,12 +1,12 @@ // UNSUPPORTED: cuda, hip, acc // FIXME: replace unsupported with an aspect check once we have it // -// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions +// RUN: %{build} -o %t.out -Xclang -fsycl-allow-virtual-functions %helper-includes // RUN: %{run} %t.out #include -#include +#include "helpers.hpp" namespace oneapi = sycl::ext::oneapi::experimental; @@ -31,62 +31,33 @@ class IncrementBy8 : public BaseIncrement { void increment(int *Data) override { *Data += 8; } }; -BaseIncrement *constructAnObject(char *Storage, int Index) { - switch (Index) { - case 0: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) BaseIncrement; - return Ret; - } - case 1: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy2; - return Ret; - } - case 2: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy4; - return Ret; - } - case 3: { - auto *Ret = reinterpret_cast(Storage); - new (Storage) IncrementBy8; - return Ret; - } - - default: - return nullptr; - } -} - int main() { - constexpr size_t Size = - std::max({sizeof(BaseIncrement), sizeof(IncrementBy2), - sizeof(IncrementBy4), sizeof(IncrementBy8)}); + using storage_t = + obj_storage_t; + + storage_t HostStorage; + sycl::buffer DeviceStorage(sycl::range{1}); - sycl::buffer ObjStorage(sycl::range{Size}); - char HostStorage[Size]; sycl::queue q; constexpr oneapi::properties props{oneapi::calls_indirectly<>}; - for (int TestCase = 0; TestCase < 4; ++TestCase) { + for (unsigned TestCase = 0; TestCase < 4; ++TestCase) { int HostData = 42; int Data = HostData; sycl::buffer DataStorage(&Data, sycl::range{1}); q.submit([&](sycl::handler &CGH) { - sycl::accessor StorageAcc(ObjStorage, CGH, sycl::write_only); + sycl::accessor StorageAcc(DeviceStorage, CGH, sycl::write_only); sycl::accessor DataAcc(DataStorage, CGH, sycl::write_only); CGH.single_task(props, [=]() { - BaseIncrement *Ptr = constructAnObject( - StorageAcc.get_multi_ptr().get(), - TestCase); + auto *Ptr = + StorageAcc[0].construct(TestCase); Ptr->increment( DataAcc.get_multi_ptr().get()); }); }); - BaseIncrement *Ptr = constructAnObject(HostStorage, TestCase); + auto *Ptr = HostStorage.construct(TestCase); Ptr->increment(&HostData); sycl::host_accessor HostAcc(DataStorage); diff --git a/sycl/test-e2e/VirtualFunctions/helpers.hpp b/sycl/test-e2e/VirtualFunctions/helpers.hpp new file mode 100644 index 0000000000000..dd51f31c1ca5f --- /dev/null +++ b/sycl/test-e2e/VirtualFunctions/helpers.hpp @@ -0,0 +1,47 @@ +#include +#include + +// TODO: strictly speaking, selecting a max alignment here may not be always +// valid, but for test cases that we have now we expect alignment of all types +// to be the same +template +struct aligned_storage_helper + : std::aligned_storage {}; + +// Helper data structure that automatically creates a right (in terms of size +// and alignment) storage to accomodate a value of any of types T... +template struct obj_storage_t { + static_assert(std::max({alignof(T)...}) == std::min({alignof(T)...}), + "Unsupported alignment of input types"); + using type = typename aligned_storage_helper::type; + static constexpr size_t size = std::max({sizeof(T)...}); + + type storage; + + template RetT *construct(const unsigned int TypeIndex) { + if (TypeIndex >= sizeof...(T)) { +#ifndef __SYCL_DEVICE_ONLY__ + assert(false && "Type index is invalid"); +#endif + return nullptr; + } + + return constructHelper(TypeIndex, 0); + } + +private: + template RetT *constructHelper(const int, const int) { + // Won't be ever called, but required to compile + return nullptr; + } + + template + RetT *constructHelper(const int TargetIndex, const int CurIndex) { + if (TargetIndex != CurIndex) + return constructHelper(TargetIndex, CurIndex + 1); + + RetT *Ptr = new (reinterpret_cast(&storage)) Type; + return Ptr; + } +}; diff --git a/sycl/test-e2e/VirtualFunctions/lit.local.cfg b/sycl/test-e2e/VirtualFunctions/lit.local.cfg new file mode 100644 index 0000000000000..f74079fb0725a --- /dev/null +++ b/sycl/test-e2e/VirtualFunctions/lit.local.cfg @@ -0,0 +1,6 @@ +import os + +# Tests are sharing some common header, but we don't won't to use relative +# paths like "../../../helper.hpp" in them, so let's just register a +# substitution to add directory with helper headers into include search path +config.substitutions.append(("%helper-includes", "-I {}".format(os.path.dirname(os.path.abspath(__file__))))) From 73d31b9c89619066bffb44efdcd349a0a1a5cedb Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Wed, 17 Jul 2024 04:58:33 -0700 Subject: [PATCH 6/6] Fix issues on windows and improve error reporting --- .../2/1/1/missing-overrides.cpp | 23 +++++++++++++++---- .../2/1/1/more-complex-hierarchy.cpp | 14 +++++++++-- .../2/1/1/simple-hierarchy.cpp | 14 +++++++++-- sycl/test-e2e/VirtualFunctions/helpers.hpp | 18 ++++++++++----- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp index 6ae83310333d5..c55847b9e2735 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/missing-overrides.cpp @@ -8,18 +8,23 @@ #include "helpers.hpp" +#include + namespace oneapi = sycl::ext::oneapi::experimental; class Base { public: SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) - virtual void increment(int *) { /* do nothhing */ } + virtual void increment(int *) { /* do nothhing */ + } SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) - virtual void multiply(int *) { /* do nothhing */ } + virtual void multiply(int *) { /* do nothhing */ + } SYCL_EXT_ONEAPI_FUNCTION_PROPERTY(oneapi::indirectly_callable<>) - virtual void substract(int *) { /* do nothhing */ } + virtual void substract(int *) { /* do nothhing */ + } }; class IncrementBy1 : public Base { @@ -64,14 +69,19 @@ void applyOp(int *DataPtr, Base *ObjPtr) { ObjPtr->multiply(DataPtr); } -int main() { +int main() try { using storage_t = obj_storage_t; storage_t HostStorage; sycl::buffer DeviceStorage(sycl::range{1}); - sycl::queue q; + auto asyncHandler = [](sycl::exception_list list) { + for (auto &e : list) + std::rethrow_exception(e); + }; + + sycl::queue q(asyncHandler); constexpr oneapi::properties props{oneapi::calls_indirectly<>}; for (unsigned TestCase = 0; TestCase < 6; ++TestCase) { @@ -97,4 +107,7 @@ int main() { } return 0; +} catch (sycl::exception &e) { + std::cout << "Unexpected exception was thrown: " << e.what() << std::endl; + return 1; } diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp index e8b86bae7481a..fce036b890294 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/more-complex-hierarchy.cpp @@ -8,6 +8,8 @@ #include "helpers.hpp" +#include + namespace oneapi = sycl::ext::oneapi::experimental; class AbstractOp { @@ -46,14 +48,19 @@ class IncrementBy8 : public IncrementOp { void applyOp(int *Data, AbstractOp *Obj) { Obj->applyOp(Data); } -int main() { +int main() try { using storage_t = obj_storage_t; storage_t HostStorage; sycl::buffer DeviceStorage(sycl::range{1}); - sycl::queue q; + auto asyncHandler = [](sycl::exception_list list) { + for (auto &e : list) + std::rethrow_exception(e); + }; + + sycl::queue q(asyncHandler); constexpr oneapi::properties props{oneapi::calls_indirectly<>}; for (unsigned TestCase = 0; TestCase < 4; ++TestCase) { @@ -80,4 +87,7 @@ int main() { } return 0; +} catch (sycl::exception &e) { + std::cout << "Unexpected exception was thrown: " << e.what() << std::endl; + return 1; } diff --git a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp index 98ca012faa044..83ec49ee3482d 100644 --- a/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp +++ b/sycl/test-e2e/VirtualFunctions/2/1/1/simple-hierarchy.cpp @@ -8,6 +8,8 @@ #include "helpers.hpp" +#include + namespace oneapi = sycl::ext::oneapi::experimental; class BaseIncrement { @@ -31,14 +33,19 @@ class IncrementBy8 : public BaseIncrement { void increment(int *Data) override { *Data += 8; } }; -int main() { +int main() try { using storage_t = obj_storage_t; storage_t HostStorage; sycl::buffer DeviceStorage(sycl::range{1}); - sycl::queue q; + auto asyncHandler = [](sycl::exception_list list) { + for (auto &e : list) + std::rethrow_exception(e); + }; + + sycl::queue q(asyncHandler); constexpr oneapi::properties props{oneapi::calls_indirectly<>}; for (unsigned TestCase = 0; TestCase < 4; ++TestCase) { @@ -65,4 +72,7 @@ int main() { } return 0; +} catch (sycl::exception &e) { + std::cout << "Unexpected exception was thrown: " << e.what() << std::endl; + return 1; } diff --git a/sycl/test-e2e/VirtualFunctions/helpers.hpp b/sycl/test-e2e/VirtualFunctions/helpers.hpp index dd51f31c1ca5f..7c5748182e6ea 100644 --- a/sycl/test-e2e/VirtualFunctions/helpers.hpp +++ b/sycl/test-e2e/VirtualFunctions/helpers.hpp @@ -3,18 +3,24 @@ // TODO: strictly speaking, selecting a max alignment here may not be always // valid, but for test cases that we have now we expect alignment of all types -// to be the same -template -struct aligned_storage_helper - : std::aligned_storage {}; +// to be the same. +// std::aligned_storage uses double under the hood which prevents us from +// using it on some HW. Therefore we use a custom implementation. +template struct aligned_storage { + static constexpr size_t Len = std::max({sizeof(T)...}); + static constexpr size_t Align = std::max({alignof(T)...}); + + struct type { + alignas(Align) unsigned char data[Len]; + }; +}; // Helper data structure that automatically creates a right (in terms of size // and alignment) storage to accomodate a value of any of types T... template struct obj_storage_t { static_assert(std::max({alignof(T)...}) == std::min({alignof(T)...}), "Unsupported alignment of input types"); - using type = typename aligned_storage_helper::type; + using type = typename aligned_storage::type; static constexpr size_t size = std::max({sizeof(T)...}); type storage;