Skip to content

[SYCL] Disable inlining kernel lambda operator at -O0 #7578

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 6 commits into from
Dec 2, 2022

Conversation

premanandrao
Copy link
Contributor

@premanandrao premanandrao commented Nov 29, 2022

PR #6977 enabled always inlining kernel lambda operators.
This PR disables this at -O0 as it was leading to a poor
debugging experience.

PR intel#6977 enabled always inliling kernel lambda operators.
This PR disables this at -O0 as it was leading to a poor
debugging experience.
@premanandrao premanandrao requested review from a team as code owners November 29, 2022 19:36
@premanandrao
Copy link
Contributor Author

The three failing tests in the CUDA LLVM test suite:
Failed Tests (3):
SYCL :: BFloat16/bfloat16_builtins.cpp
SYCL :: Matrix/element_wise_wi_marray_legacy.cpp
SYCL :: Matrix/joint_matrix_tensorcores_legacy.cpp
seem to be bloat16 related.

@pvchupin
Copy link
Contributor

#7567 should fix fails.

@premanandrao
Copy link
Contributor Author

@pvchupin, the CUDA LLVM test suite is failing for reasons not clear to me.

@premanandrao premanandrao marked this pull request as draft December 1, 2022 14:42
@premanandrao
Copy link
Contributor Author

Temporarily moving this to draft state since it looks like passing the flag to additional components may be necessary.

@premanandrao premanandrao marked this pull request as ready for review December 1, 2022 16:30
@premanandrao
Copy link
Contributor Author

Sorry, that was a false alarm due to incorrect environment setup. This is ready for review.

@premanandrao premanandrao requested a review from hchilama December 1, 2022 16:31
Copy link
Contributor

@mdtoguchi mdtoguchi 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!

@premanandrao
Copy link
Contributor Author

@pvchupin, the tests seem to be failing again, though there were no compiler changes between yesterday (when it all succeeded) and now. (I did change a test based on code-review, and that test does pass.)

Could you please review the doc change and if you are okay with that, merge the change?

Comment on lines 115 to 116
Enabled by default; disabled when optimizations are disabled (-O0 or
equivalent).
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I remember clang has -O0 by default?

Suggested change
Enabled by default; disabled when optimizations are disabled (-O0 or
equivalent).
Disabled when optimizations are disabled (-O0 or
equivalent). Enabled otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks.

@pvchupin
Copy link
Contributor

pvchupin commented Dec 1, 2022

@premanandrao, error when toolchain archive is not found happens usually when you restart failed task manually. When you update the patch you don't need to do anything actually, testing will restart automatically. In the cases when you really want to restart testing the only option is to "Restart all tasks". The reason is that toolchain artifact is erased on the runner as soon as testing is over. Restarting just llvm-test-suite testing is not possible today.

@premanandrao
Copy link
Contributor Author

@premanandrao, error when toolchain archive is not found happens usually when you restart failed task manually. When you update the patch you don't need to do anything actually, testing will restart automatically. In the cases when you really want to restart testing the only option is to "Restart all tasks". The reason is that toolchain artifact is erased on the runner as soon as testing is over. Restarting just llvm-test-suite testing is not possible today.

Thanks, that is helpful. In my case, testing had started automatically after patch update, and that testing failed cryptically - like with a failed cloud service message. I realize now that I should not have attempted to restart as it seems the original failure info is lost.

@premanandrao
Copy link
Contributor Author

Failing tests are unrelated to this change.

@pvchupin / @intel/llvm-gatekeepers, this is ready for a merge.

@pvchupin pvchupin merged commit 2359d94 into intel:sycl Dec 2, 2022
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.

4 participants