Skip to content

[SYCL] Adjust GCC workaround and its scope #13144

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 2 commits into from
Mar 28, 2024

Conversation

frasercrmck
Copy link
Contributor

Previously we were hard-coding an -O2 optimization level for the 'signbit' builtin for all versions of GCC.

Despite this workaround, I found locally that I was unable to build with GCC versions 12.2, 12.3, and 13.2. Reducing the optimization level to -O1 allowed me to progress. This seems to follow the bug report already linked, which had test cases at -O2 which were also failing.

With this in mind, we can also restrict the GCC versions we apply the workaround to, so that more modern compilers should "just work" without us having to do anything. That should save someone having to investigate a performance report a year or so down the line...

@frasercrmck frasercrmck requested a review from a team as a code owner March 25, 2024 13:12
Previously we were hard-coding an -O2 optimization level for the
'signbit' builtin for all versions of GCC.

Despite this workaround, I found locally that I was unable to build with
GCC versions 12.2, 12.3, and 13.2. Reducing the optimization level to
-O1 allowed me to progress. This seems to follow the bug report already
linked, which had test cases at -O2 which were also failing.

With this in mind, we can also restrict the GCC versions we apply the
workaround to, so that more modern compilers should "just work" without
us having to do anything. That should save someone having to investigate
a performance report a year or so down the line...
Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

Generally LGTM, I have two minor comments that I don't have strong opinion on.

@frasercrmck
Copy link
Contributor Author

@intel/llvm-gatekeepers I think this can be merged - thanks.

@martygrant martygrant merged commit f894d08 into intel:sycl Mar 28, 2024
@frasercrmck frasercrmck deleted the update-gcc-workaround branch March 28, 2024 11:07
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