Skip to content

[ESIMD] Reduce number of bit-casts generated for lsc_block_load/store operations #8385

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
Merged
merged 12 commits into from
Feb 21, 2023
Merged

[ESIMD] Reduce number of bit-casts generated for lsc_block_load/store operations #8385

merged 12 commits into from
Feb 21, 2023

Conversation

fineg74
Copy link
Contributor

@fineg74 fineg74 commented Feb 17, 2023

No description provided.

@fineg74 fineg74 requested a review from a team as a code owner February 17, 2023 04:01
v-klochkov
v-klochkov approved these changes Feb 17, 2023
View reviewed changes
Copy link
Contributor

@v-klochkov v-klochkov left a comment

Choose a reason for hiding this comment

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

Looks good/correct. Thank you.

One comment for future changes in these functions:
It feels like it is possible to pass lsc_data_size::u32 and T=double and not get static verification/compilation error. This needs to be fixed.

@fineg74 fineg74 temporarily deployed to aws February 17, 2023 04:27 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 17, 2023 04:36 — with GitHub Actions Inactive
@fineg74
Copy link
Contributor Author

fineg74 commented Feb 17, 2023

Test failures are related to infrastructure, not the change

@fineg74 fineg74 temporarily deployed to aws February 17, 2023 05:59 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 17, 2023 06:30 — with GitHub Actions Inactive
@fineg74
Copy link
Contributor Author

fineg74 commented Feb 17, 2023

Looks good/correct. Thank you.

One comment for future changes in these functions: It feels like it is possible to pass lsc_data_size::u32 and T=double and not get static verification/compilation error. This needs to be fixed.

This is true for all the lsc API we have, not just these functions so it is better be done as a separate item.

@fineg74 fineg74 temporarily deployed to aws February 17, 2023 23:17 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 17, 2023 23:48 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 18, 2023 00:42 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 18, 2023 01:54 — with GitHub Actions Inactive
@bader
Copy link
Contributor

bader commented Feb 18, 2023

@fineg74, I restarted pre-commit checks. OpenCL CPU and ESIMD emulator results were invalid due to infrastructure issue. Sorry for inconvenience.

@fineg74 fineg74 temporarily deployed to aws February 18, 2023 02:52 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 18, 2023 05:22 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 18, 2023 05:54 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 18, 2023 23:56 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws February 19, 2023 00:28 — with GitHub Actions Inactive
v-klochkov
v-klochkov reviewed Feb 21, 2023
View reviewed changes
sycl/include/sycl/ext/intel/experimental/esimd/detail/memory_intrin.hpp
@@ -420,7 +419,12 @@ void __esimd_emu_lsc_offset_write(
VecIdx += vectorIndexIncrement<N, _Transposed>()) {

if ((ByteDistance >= 0) && (ByteDistance < BufByteWidth)) {
*((StoreType *)(WriteBase + ByteDistance)) = vals[VecIdx];
if constexpr (std::is_floating_point<Ty>::value) {
*((StoreType *)(WriteBase + ByteDistance)) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't this branch be used unconditionally: i.e. for if-then and if-else?

@fineg74 fineg74 changed the title [SYCL][ESIMD] Fix an issue with incorrect loading/storing of floating point data under some circumstances [ESIMD] Reduce number of bit-casts generated for lsc_block_load/store operations Feb 21, 2023
@v-klochkov v-klochkov merged commit 039b538 into intel:sycl Feb 21, 2023
@fineg74 fineg74 deleted the doubleFix branch February 21, 2023 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v-klochkov v-klochkov v-klochkov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fineg74 @bader @v-klochkov