Skip to content
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.

Sign up for GitHub

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

Jump to bottom

clang generates definitions of sqrt #1231

Closed
alan-baker opened this issue Sep 14, 2023 · 0 comments · Fixed by #1271
Closed

clang generates definitions of sqrt #1231

alan-baker opened this issue Sep 14, 2023 · 0 comments · Fixed by #1271
Assignees
@alan-baker

Comments

@alan-baker
Copy link
Collaborator

https://reviews.llvm.org/D156743 added definitions of float-based sqrt to the opencl header. This leads to problems in clspv:

  • long vectors are always present
  • cllib linking is broken

Short term solution is to enable long vectors and strip builtin definitions prior to linking.
@alan-baker alan-baker self-assigned this Sep 14, 2023
alan-baker added a commit to alan-baker/clspv that referenced this issue Sep 14, 2023
@alan-baker
Update deps
7c885f1 
Contributes to google#1231

* Update llvm, spirv-headers, and spirv-tools
* Clang defines sqrt for float in opencl-c-base.h now
  * need to strip it to facilitate linking with cllib
  * need to always enable long vector lowering
    * xfail'd some tests for now
* Add missing dependency for header generation on opencl-c-base.h
@alan-baker alan-baker mentioned this issue Sep 14, 2023
Merged
alan-baker added a commit that referenced this issue Sep 14, 2023
@alan-baker
Update deps (#1232)
c0b1c96 
Contributes to #1231

* Update llvm, spirv-headers, and spirv-tools
* Clang defines sqrt for float in opencl-c-base.h now
  * need to strip it to facilitate linking with cllib
  * need to always enable long vector lowering
    * xfail'd some tests for now
* Add missing dependency for header generation on opencl-c-base.h
@rjodinchr rjodinchr mentioned this issue Sep 18, 2023
Merged
arsenm pushed a commit to llvm/llvm-project that referenced this issue Dec 1, 2023
@rjodinchr
clang/OpenCL: set sqrt fp accuracy on call to Z4sqrt (#66651)
d56e0d0 
This is reverting the previous implementation to avoid adding inline
function in opencl headers.
This was breaking clspv flow google/clspv#1231, while
https://reviews.llvm.org/D156743 mentioned that just decorating the call
node with `!pfmath` was enough.
This PR is implementing this idea.
The test has been updated with this implementation.
rjodinchr added a commit to rjodinchr/clspv that referenced this issue Dec 11, 2023
@rjodinchr
disable long-vector by default now that llvm has been fixed
4d8f9f4 
since llvm/llvm-project#66651 has been merged
we can revert part of google#1232.

Fix google#1231
@rjodinchr rjodinchr mentioned this issue Dec 11, 2023
Merged
alan-baker pushed a commit that referenced this issue Dec 11, 2023
@rjodinchr
disable long-vector by default now that llvm has been fixed (#1271)
6960f8d 
since llvm/llvm-project#66651 has been merged
we can revert part of #1232.

Fix #1231
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alan-baker alan-baker

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

disable long-vector by default now that llvm has been fixed rjodinchr/clspv
1 participant
@alan-baker