-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL] Add support for __arithmetic_fence builtin for SYCL targets. #8072
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One NIT, otherwise lgtm
@intel/llvm-gatekeepers I don't think the failure is related to this PR. In which case the PR is ready to merge. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we are doing this in intel/llvm and not community?
I can't think of any reason. |
I would suggest attempting to put this in community instead then. We always try to do that if possible. |
I just verified and sycl_device attribute is not implemented. |
|
@zahiraam, could you add tag(s) for the PR title, please? https://github.com/intel/llvm/blob/sycl/CONTRIBUTING.md#pull-request
SYCL or SPIR? |
I suggest following @elizabethandrews suggestion and commit this change to llvm.org. A couple of notes:
|
__arithmetic_fence enforces ordering on expression evaluation when fast-math is enabled.
In fast math mode some floating-point optimizations are performed such as reassociation and distribution.
For example, the compiler may transform (a+b)+c into a+(b+c). Although these two expressions are equivalent in integer arithmetic, they may not be in floating-point arithmetic. The builtin tells the compiler that the expression in parenthesis can’t be re-associated or distributed.
__arithmetic_fence(a+b)+c is not equivalent to a+(b+c).