Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SYCL] test update: support single tile PVC #12716

Closed
wants to merge 2 commits into from
Closed

[SYCL] test update: support single tile PVC #12716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a231ca5
checkpoint. work not yet finished.
cperkinsintel Feb 14, 2024
ba058de
Some of our PVC tests were assuming multiple tiles, but that is not t…
cperkinsintel Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions sycl/test-e2e/Plugin/level_zero_ext_intel_cslice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,20 @@ bool IsPVC(device &d) {
return masked_device_id == 0xbd0 || masked_device_id == 0xb60;
}

bool IsPVC_MultiTiles(device &d) {
// PVC-1T (one tile) does not support partitioning by affinity domain,
// which this test requires
if (!isPartitionableByAffinityDomain(d))
return false;

return IsPVC(d);
}

void test_pvc(device &d) {
std::cout << "Test PVC Begin" << std::endl;
// CHECK-PVC: Test PVC Begin
std::cout << "IsPVC: " << IsPVC(d) << std::endl;
if (IsPVC(d)) {
std::cout << "IsPVC: " << IsPVC_MultiTiles(d) << std::endl;
if (IsPVC_MultiTiles(d)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should adjust the test to check that even 1T cards can be partitioned by CSlice.

assert(isPartitionableByAffinityDomain(d));
assert(!isPartitionableByCSlice(d));
{
Expand Down Expand Up @@ -146,7 +155,7 @@ void test_pvc(device &d) {
}

void test_non_pvc(device &d) {
if (IsPVC(d))
if (IsPVC_MultiTiles(d))
return;

// Non-PVC devices are not partitionable by CSlice at any level of
Expand Down
35 changes: 30 additions & 5 deletions sycl/test-e2e/Plugin/level_zero_ext_intel_queue_index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,39 @@

using namespace sycl;

template <typename RangeTy, typename ElemTy>
bool contains(RangeTy &&Range, const ElemTy &Elem) {
return std::find(Range.begin(), Range.end(), Elem) != Range.end();
}

bool isPartitionableBy(device &Dev, info::partition_property Prop) {
return contains(Dev.get_info<info::device::partition_properties>(), Prop);
}

bool isPartitionableByAffinityDomain(device &Dev) {
return isPartitionableBy(
Dev, info::partition_property::partition_by_affinity_domain);
}

bool IsPVC(device &d) {
uint32_t masked_device_id =
d.get_info<ext::intel::info::device::device_id>() & 0xff0;
return masked_device_id == 0xbd0 || masked_device_id == 0xb60;
}

bool IsPVC_MultiTiles(device &d) {
// PVC-1T (one tile) does not support partitioning by affinity domain,
// which this test requires
Comment on lines +34 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should support it in SYCL_PI_LEVEL_ZERO_EXPOSE_CSLICE_IN_AFFINITY_PARTITIONING=1 mode still, shouldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno. But if get_info<> returns that it is supported, then the test should be correct regardless.

if (!isPartitionableByAffinityDomain(d))
return false;

return IsPVC(d);
}

void test_pvc(device &d) {
std::cout << "Test PVC Begin" << std::endl;
// CHECK-PVC: Test PVC Begin
bool IsPVC = [&]() {
uint32_t masked_device_id =
d.get_info<ext::intel::info::device::device_id>() & 0xff0;
return masked_device_id == 0xbd0 || masked_device_id == 0xb60;
}();
bool IsPVC = IsPVC_MultiTiles(d);
std::cout << "IsPVC: " << std::boolalpha << IsPVC << std::endl;
if (IsPVC) {
assert(d.get_info<ext::intel::info::device::max_compute_queue_indices>() ==
Expand Down
38 changes: 38 additions & 0 deletions sycl/test-e2e/Plugin/level_zero_sub_sub_device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
// REQUIRES: gpu-intel-pvc, level_zero
// REQUIRES: aspect-ext_intel_device_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need that? We already have REQUIRES: gpu-intel-pvc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. I was keeping the IsPVC() test the same for all three of these and it retrieves the device_id.


// RUN: %{build} %level_zero_options -o %t.out
// RUN: env UR_L0_DEBUG=1 env ZEX_NUMBER_OF_CCS=0:4 %{run} %t.out 2>&1 | FileCheck %s
// RUN: env ZEX_NUMBER_OF_CCS=0:4 %{run} %t.out

// Check that queues created on sub-sub-devices are going to specific compute
// engines:
auto constexpr expected_output = R"===(
// CHECK: [getZeQueue]: create queue ordinal = 0, index = 0 (round robin in [0, 0])
// CHECK: [getZeQueue]: create queue ordinal = 0, index = 1 (round robin in [1, 1])
// CHECK: [getZeQueue]: create queue ordinal = 0, index = 2 (round robin in [2, 2])
// CHECK: [getZeQueue]: create queue ordinal = 0, index = 3 (round robin in [3, 3])
)===";

#include <chrono>
#include <cmath>
Expand All @@ -21,6 +24,35 @@
using namespace sycl;
using namespace std::chrono;

template <typename RangeTy, typename ElemTy>
bool contains(RangeTy &&Range, const ElemTy &Elem) {
return std::find(Range.begin(), Range.end(), Elem) != Range.end();
}

bool isPartitionableBy(device &Dev, info::partition_property Prop) {
return contains(Dev.get_info<info::device::partition_properties>(), Prop);
}

bool isPartitionableByAffinityDomain(device &Dev) {
return isPartitionableBy(
Dev, info::partition_property::partition_by_affinity_domain);
}

bool IsPVC(device &d) {
uint32_t masked_device_id =
d.get_info<ext::intel::info::device::device_id>() & 0xff0;
return masked_device_id == 0xbd0 || masked_device_id == 0xb60;
}

bool IsPVC_MultiTiles(device &d) {
// PVC-1T (one tile) does not support partitioning by affinity domain,
// which this test requires
if (!isPartitionableByAffinityDomain(d))
return false;

return IsPVC(d);
}

#define random_float() (rand() / double(RAND_MAX))
#define INTER_NUM (150)
#define KERNEL_NUM (2000)
Expand Down Expand Up @@ -91,6 +123,12 @@ int main(void) {

device d;

// PVC-1T does not support partition by affinity domain
if (!IsPVC_MultiTiles(d)) {
std::cout << expected_output << std::endl; // trick FileCheck
return 0;
}

Comment on lines +126 to +131
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that if we stop supporting multi-tile cards, the test would keep passing. Do we know if PVC-1T and PVC-2T differ in their device id? If so, can we use that instead of isPartitionableByAffinityDomain query?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have access to the a PVC-1T, but the IsPVC() test retrieves the device_id and verifies it with a mask. It is returning true for both PVC-1T and PVC-2T. It's possible that they differ in the range of that mask, but I haven't verified that by hand. But rather than key off of some poorly understood device id, I think it's better to do as we are here: actually check that the the AffinityDomain partitioning is available. The test needs it, so its correct to check it.

// watch out device here
auto subdevices = d.create_sub_devices<
info::partition_property::partition_by_affinity_domain>(
Expand Down
Loading