Skip to content

[SYCL] Add support for assuming SYCL queries fit in int #2215

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.

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

Merged
merged 11 commits into from
Aug 13, 2020

Conversation

mdtoguchi
Copy link
Contributor

For good default performance, compiler needs to set
-D_SYCL_ID_QUERIES_FIT_IN_INT_ by default. This will add
__builtin_assume() that says global_id/linear_id fits within
MAX_INT. We also need to have an ability to override where
programmer needs 31+bit in global_id/linear_id. This is
provided via -f[no-]sycl-id-queries-fit-in-int

AGindinson
AGindinson previously approved these changes Jul 30, 2020
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

@mdtoguchi
Copy link
Contributor Author

@mdtoguchi should it also be enabled for https://github.com/intel/llvm/blob/sycl/sycl/test/check_device_code/id_queries_fit_int.cpp?

@alexbatashev, looks like the test can be updated to use the formal option instead of the predefine directly. I can make that adjustment.

@mdtoguchi mdtoguchi force-pushed the private/mdtoguchi/sycl-id-queries-in-int branch from 72560ad to ddc0934 Compare July 30, 2020 18:59
@mdtoguchi mdtoguchi requested a review from a team as a code owner July 30, 2020 18:59
@mdtoguchi mdtoguchi requested a review from s-kanaev July 30, 2020 18:59
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

@mdtoguchi
Copy link
Contributor Author

Made updates to not have this behavior on by default.

@mdtoguchi mdtoguchi force-pushed the private/mdtoguchi/sycl-id-queries-in-int branch from 1741eed to fbf863a Compare August 2, 2020 19:33
@mdtoguchi mdtoguchi requested a review from AGindinson August 5, 2020 16:57
@mdtoguchi mdtoguchi force-pushed the private/mdtoguchi/sycl-id-queries-in-int branch from 47fdcb2 to 0e323c7 Compare August 7, 2020 16:43
s-kanaev
s-kanaev previously approved these changes Aug 10, 2020
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

RT changes LGTM.

@mdtoguchi
Copy link
Contributor Author

@AGindinson , @premanandrao , could I get reviews?

alexbatashev
alexbatashev previously approved these changes Aug 12, 2020
Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

LGTM

s-kanaev
s-kanaev previously approved these changes Aug 12, 2020
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

RT changes LGTM.

@mdtoguchi mdtoguchi dismissed stale reviews from s-kanaev and alexbatashev via 25298d9 August 12, 2020 15:33
Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

Last question I promise :) Sorry I didn't notice this before. Otherwise LGTM

Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@mdtoguchi mdtoguchi force-pushed the private/mdtoguchi/sycl-id-queries-in-int branch from 25298d9 to fe2ab8c Compare August 13, 2020 02:07
@bader bader merged commit 3e4da3c into intel:sycl Aug 13, 2020
jsji pushed a commit that referenced this pull request Nov 16, 2023
https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179 is being implemented, and some tests become obsolete.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@2075517
Chenyang-L pushed a commit that referenced this pull request Feb 18, 2025
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.

7 participants