Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL][CUDA] Add unit test for local arguments alignment #608

Merged
merged 2 commits into from
Mar 4, 2022

Conversation

npmiller
Copy link

This issue was solved in intel/llvm#5113, local kernel arguments have to
be aligned to the type size.

cc @romanovvlad

This issue was solved in intel/llvm#5113, local kernel arguments have to
be aligned to the type size.
@vladimirlaz
Copy link

/verify with intel/llvm#5113

@vladimirlaz vladimirlaz self-requested a review December 12, 2021 15:58
@vladimirlaz
Copy link

@npmiller it looks like the test is failing on Linux (OpenCL BE). I have restarted it with the latest compiler.

Copy link

@vladimirlaz vladimirlaz left a comment

Choose a reason for hiding this comment

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

@npmiller could you please fix CI issue reported for OpenCL BE.

[2022-02-10T07:00:45.849Z] ******************** TEST 'SYCL :: Regression/local-arg-align.cpp' FAILED ********************
[2022-02-10T07:00:45.849Z] Script:
[2022-02-10T07:00:45.849Z] --
[2022-02-10T07:00:45.849Z] : 'RUN: at line 1';    /runDir/jenkins-dir/workspace/llvm.obj/bin/clang++      -fsycl -fsycl-targets=spir64 /runDir/jenkins-dir/workspace/llvm-test-suite/SYCL/Regression/local-arg-align.cpp -o /runDir/jenkins-dir/workspace/build/SYCL/Regression/Output/local-arg-align.cpp.tmp.out
[2022-02-10T07:00:45.849Z] : 'RUN: at line 3';   env SYCL_DEVICE_FILTER=host  /runDir/jenkins-dir/workspace/build/SYCL/Regression/Output/local-arg-align.cpp.tmp.out
[2022-02-10T07:00:45.849Z] : 'RUN: at line 4';   env SYCL_DEVICE_FILTER=opencl:cpu,host  /runDir/jenkins-dir/workspace/build/SYCL/Regression/Output/local-arg-align.cpp.tmp.out
[2022-02-10T07:00:45.849Z] : 'RUN: at line 5';    env SYCL_DEVICE_FILTER=opencl:gpu,host  /runDir/jenkins-dir/workspace/build/SYCL/Regression/Output/local-arg-align.cpp.tmp.out
[2022-02-10T07:00:45.849Z] : 'RUN: at line 6';    env SYCL_DEVICE_FILTER=acc  /runDir/jenkins-dir/workspace/build/SYCL/Regression/Output/local-arg-align.cpp.tmp.out
[2022-02-10T07:00:45.849Z] --
[2022-02-10T07:00:45.849Z] Exit Code: 255
[2022-02-10T07:00:45.849Z] 
[2022-02-10T07:00:45.849Z] Command Output (stdout):
[2022-02-10T07:00:45.849Z] --
[2022-02-10T07:00:45.849Z] $ ":" "RUN: at line 1"
[2022-02-10T07:00:45.849Z] note: command had no output on stdout or stderr
[2022-02-10T07:00:45.849Z] $ "/runDir/jenkins-dir/workspace/llvm.obj/bin/clang++" "-fsycl" "-fsycl-targets=spir64" "/runDir/jenkins-dir/workspace/llvm-test-suite/SYCL/Regression/local-arg-align.cpp" "-o" "/runDir/jenkins-dir/workspace/build/SYCL/Regression/Output/local-arg-align.cpp.tmp.out"
[2022-02-10T07:00:45.849Z] note: command had no output on stdout or stderr
[2022-02-10T07:00:45.849Z] $ ":" "RUN: at line 3"
[2022-02-10T07:00:45.849Z] note: command had no output on stdout or stderr
[2022-02-10T07:00:45.849Z] $ "env" "SYCL_DEVICE_FILTER=host" "/runDir/jenkins-dir/workspace/build/SYCL/Regression/Output/local-arg-align.cpp.tmp.out"
[2022-02-10T07:00:45.849Z] # command output:
[2022-02-10T07:00:45.849Z] Error: incorrect alignment for argument b, required alignment: 32, address: 0x1af3730
[2022-02-10T07:00:45.849Z] 
[2022-02-10T07:00:45.849Z] error: command failed with exit status: 255
[2022-02-10T07:00:45.849Z] 
[2022-02-10T07:00:45.849Z] --
[2022-02-10T07:00:45.849Z] 
[2022-02-10T07:00:45.849Z] ********************

@npmiller
Copy link
Author

Oh that's interesting, do you have more details on what OpenCL runtime and device is being used in the CI?

I've had another look through the test, and I can't really see anything wrong with it, I think this is likely a problem in the OpenCL runtime, or potentially how the SPIR-V binaries are generated, I'll try to see if I can track it down a bit further.

@npmiller
Copy link
Author

I'm actually able to reproduce this with the Nvidia OpenCL so I reckon this is likely what the CI also uses.

@npmiller
Copy link
Author

Okay, so I have tracked this down, it's actually not an issue with OpenCL at all, it's a host problem.

Local accessors for host are allocated as std::vector<char>, so they might be misaligned:

Which is exactly what happens in this test. Replacing this with an aligned allocation on MElemSize I get the test to pass, I'm looking into submitting a patch for this, however it does break the ABI which is not ideal.

bader pushed a commit to intel/llvm that referenced this pull request Mar 1, 2022
Local kernel arguments must be aligned to the type size, simply using `std::vector<char>` doesn't always provide the correct alignment. So this patch adds extra padding to the vector and ensures that the pointer returned for the accessor is actually aligned to the type size.

This issue was exposed by: intel/llvm-test-suite#608, which was a follow up to fixing local accessor alignment for the CUDA plugin.
@npmiller
Copy link
Author

npmiller commented Mar 1, 2022

The failing test should be fixed now that intel/llvm#5554 was merged

@vladimirlaz
Copy link

will wait till next compiler nightly to ensure that the test will pass with it before merge

@bader bader merged commit 32fbf01 into intel:intel Mar 4, 2022
myler added a commit to myler/llvm-test-suite that referenced this pull request Apr 12, 2022
CMPLRTST-15545: fixed git related issue, test driver is able to identify good commit.
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Apr 12, 2022
This issue was solved in intel/llvm#5113, local kernel arguments have to
be aligned to the type size.
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…test-suite#608)

This issue was solved in intel#5113, local kernel arguments have to
be aligned to the type size.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants