[SYCL] test update: support single tile PVC #12716
Conversation
…he case in all installations.
cperkinsintel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
cperkinsintel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| std::cout << "IsPVC: " << IsPVC(d) << std::endl; | ||
| if (IsPVC(d)) { | ||
| std::cout << "IsPVC: " << IsPVC_MultiTiles(d) << std::endl; | ||
| if (IsPVC_MultiTiles(d)) { |
There was a problem hiding this comment.
I think we should adjust the test to check that even 1T cards can be partitioned by CSlice.
| // PVC-1T (one tile) does not support partitioning by affinity domain, | ||
| // which this test requires |
There was a problem hiding this comment.
It should support it in SYCL_PI_LEVEL_ZERO_EXPOSE_CSLICE_IN_AFFINITY_PARTITIONING=1 mode still, shouldn't it?
There was a problem hiding this comment.
Dunno. But if get_info<> returns that it is supported, then the test should be correct regardless.
| @@ -1,15 +1,18 @@ | |||
| // REQUIRES: gpu-intel-pvc, level_zero | |||
| // REQUIRES: aspect-ext_intel_device_id | |||
There was a problem hiding this comment.
Why do we need that? We already have REQUIRES: gpu-intel-pvc.
There was a problem hiding this comment.
True. I was keeping the IsPVC() test the same for all three of these and it retrieves the device_id.
| // PVC-1T does not support partition by affinity domain | ||
| if (!IsPVC_MultiTiles(d)) { | ||
| std::cout << expected_output << std::endl; // trick FileCheck | ||
| return 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
I'm wondering if the code-owner for files should be the L0 team? I believe the intent of UR team being made code-owner for this test directory is that it would eventually be superceeded by UR testing but I'm not sure if that makes sense for these tests or not? |
When the feature was first implemented, most of the logic was in plugins/level_zero/pi_level_zero.cpp (https://github.com/intel/llvm/pull/7626/files#diff-15dd1eb076d2164bd9e87d9057f05f652a716498e8cdf5975e564c65309a0985). I believe that that code belongs to UR and as such the test should be owned by it. |
|
closing in favor of #13067 |
A few of our tests assume that a given PVC installation always has multiple tiles available, which is not true. Putting appropriate guards into those tests.