Skip to content

Conversation

@alexbatashev
Copy link
Contributor

The SYCL specification mentions load and store member functions as

template <access::address_space addressSpace>
void load(size_t offset, multi_ptr<const dataT, addressSpace> ptr);
template <access::address_space addressSpace>
void store(size_t offset, multi_ptr<dataT, addressSpace> ptr) const;

This patch aligns our implementation with the definition that is provided by the spec.

Signed-off-by: Alexander Batashev alexander.batashev@intel.com

Comment on lines +659 to +665
Copy link
Contributor

Choose a reason for hiding this comment

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

Cannot find such overload in the spec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexeySachkov but it is being used in CTS

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexbatashev, then it is either CTS bug or spec bug.

Tagging @bader, @mkinsner and @jbrodman for visibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexeySachkov I submitted an issue to track this question KhronosGroup/SYCL-CTS#36

Comment on lines +672 to +677
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
@alexbatashev alexbatashev force-pushed the private/abatashe/fix_vector_load_store branch from 379f7dd to 7547a46 Compare December 19, 2019 14:29
@alexbatashev alexbatashev changed the title [SYCL] Align vec with spec [SYCL] Make vec load store member functions templated Dec 19, 2019
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

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

Overall, I don't have objections against the patch. Situation with extra overload seems to me like a spec bug because this overload is used in official conformance tests.

@romanovvlad romanovvlad merged commit 4bd76de into intel:sycl Dec 24, 2019
vmaksimo pushed a commit to vmaksimo/llvm that referenced this pull request Mar 5, 2021
There's no real difference at the SPIR-V level between these and the
more generic mem_fence intrinsic because mem_fence is already relaxed
and SPIR-V doesn't have a concept of anything more relaxed than that.

Closes: intel#920

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@a53d233
vladimirlaz pushed a commit that referenced this pull request Mar 30, 2021
There's no real difference at the SPIR-V level between these and the
more generic mem_fence intrinsic because mem_fence is already relaxed
and SPIR-V doesn't have a concept of anything more relaxed than that.

Closes: #920

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@a53d233
@alexbatashev alexbatashev deleted the private/abatashe/fix_vector_load_store branch July 28, 2021 06:44
coldav pushed a commit to coldav/llvm that referenced this pull request Aug 22, 2025
…tuff

[NFC] Remove more old debug info handling.
coldav pushed a commit to coldav/llvm that referenced this pull request Aug 26, 2025
…tuff

[NFC] Remove more old debug info handling.
coldav pushed a commit to coldav/llvm that referenced this pull request Aug 29, 2025
…tuff

[NFC] Remove more old debug info handling.
jsji added a commit that referenced this pull request Nov 4, 2025
…14b not in (#920)

5e0f14b was reverted in 1a21f73.
The test was added in c4fd801.
jsji added a commit that referenced this pull request Nov 6, 2025
…14b not in (#920)

5e0f14b was reverted in 1a21f73.
The test was added in c4fd801.
jsji added a commit that referenced this pull request Nov 6, 2025
…14b not in (#920)

5e0f14b was reverted in 1a21f73.
The test was added in c4fd801.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants