[SYCL] Fix 2D slicepitch info #18960
Conversation
Make slicepitch image size and num slices 1 for a 2D image.
|
Can we have a test for this? |
|
I could potentially make a unittest that checks whether a valid slice pitch is passed to the backend. The slice pitch is not exposed to the user for 2D images so the test would need to look at a backend |
unittest works for me! Thanks! |
Make sure the SYCL runtime is passing correct values to UR layer.
|
I've added a test in the E2E tests. There weren't any |
Don't run on L0 and remove the std::cout at the end.
Don't include read write flags, these may differ per backend.
Pitches are only used when host ptr is non null.
Pitches are configured differently if a new allocation is made.
If a host ptr is used, exit early. This means that FileCheck should only be invoked if the test doesn't exit early. This conditional is best expressed in Python as it is portable across Windows and Linux.
hdelan
force-pushed
the
image-pitch-bugfix
branch
from
25f99f2 to
f62a8aa
Compare
|
@intel/llvm-gatekeepers the failure is unrelated. Can we merge this please? |
|
@hdelan The failure is related, you need to add |
And this is the reason why simply saying "failure is unrelated" without even copying the failing test name into the comment is wrong. |
| // This test should be allowed to exit early if the image doesn't use a host | ||
| // ptr, but it should pipe stdout to FileCheck otherwise. A portable way to | ||
| // allow an early exit without any output to pipe to FileCheck logic is to use | ||
| // Python. | ||
| // RUN: env SYCL_UR_TRACE=-1 %{run} python -c "import subprocess, sys; p = subprocess.run(['%t.out'], stdout=subprocess.PIPE, check=True); o = p.stdout.strip(); sys.exit(0) if not o else subprocess.run(['FileCheck', '%s'], input=o, text=True).returncode" |
There was a problem hiding this comment.
Wow... And I still have no clue why is this necessary after reading this comment.
There was a problem hiding this comment.
Yeah, IMO we really need to avoid this. We should split the test if there's no way to do it in a single test.
I do it too something :(, I'll try to be better both myself and checking for it before merging :) |
| // Trace will be different depending on whether host ptr is used or not | ||
| if (!Img1.has_property<sycl::property::image::use_host_ptr>()) { |
There was a problem hiding this comment.
Who sets this property?
|
@hdelan Please don't do 9 commits after approval without asking for it again. I'm happy to review as many times as needed. |
Make
slicePitchequal to image size andnumSlices1 for a 2D image.This was incorrect but wasn't failing as many backends ignore
Pitch[1]for a 2D image.