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

[SYCL][Fusion] Pass -O2 flag to tests performing argument promotion #1601

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

victor-eds
Copy link

No promotion will take place if -O0 is passed, so add this flag to override options possibly disrupting tests results.

No promotion will take place if -O0 is passed, so add this flag to
override options possibly disrupting tests results.

Signed-off-by: Victor Perez <[email protected]>
@victor-eds victor-eds requested a review from a team as a code owner February 16, 2023 15:00
@victor-eds victor-eds requested a review from bso-intel February 16, 2023 15:00
@victor-eds victor-eds self-assigned this Feb 16, 2023
Copy link

@sommerlukas sommerlukas left a comment

Choose a reason for hiding this comment

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

LGTM.

@victor-eds victor-eds marked this pull request as draft February 23, 2023 10:39
@victor-eds victor-eds marked this pull request as ready for review March 23, 2023 10:34
@steffenlarsen
Copy link

We recently saw cases where -O flags were ignored on Windows because the test-suite uses clang-cl. As such I worry this only avoid the problem on Linux. A more stable solution could be to introduce new LIT variables picking -O2 or /O2 depending on the target.

@sommerlukas
Copy link

We recently saw cases where -O flags were ignored on Windows because the test-suite uses clang-cl. As such I worry this only avoid the problem on Linux. A more stable solution could be to introduce new LIT variables picking -O2 or /O2 depending on the target.

Note that fusion is currently only supported on Linux. On Windows, the LIT configuration feature fusion is not available, so all the tests with the -O flag should be marked unsupported by the REQUIRES: fusion clause.

Copy link

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Note that fusion is currently only supported on Linux. On Windows, the LIT configuration feature fusion is not available, so all the tests with the -O flag should be marked unsupported by the REQUIRES: fusion clause.

Point taken! In that case, I am okay with this. If Windows support is added in the future, please keep my previous comment in mind. 😄

@steffenlarsen steffenlarsen merged commit 574cee1 into intel:intel Mar 23, 2023
@victor-eds victor-eds deleted the add-o2-to-intern-tests branch March 23, 2023 14:52
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…ntel/llvm-test-suite#1601)

No promotion will take place if -O0 is passed, so add this flag to
override options possibly disrupting tests results.

Signed-off-by: Victor Perez <[email protected]>
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.

3 participants