Skip to content

Conversation

@jinge90
Copy link
Contributor

@jinge90 jinge90 commented Dec 11, 2025

There are duplicates for _imf* declarations in sycl/builtins.hpp, this PR aims to remove them, the necessary declarations will be moved into sycl/ext/intel/math.hpp to make sycl builtins.hpp clean.
We also unify the style for _imf* declarations in sycl intel math header to move them close to where they are invoked.

@jinge90 jinge90 requested a review from a team as a code owner December 11, 2025 04:41
@jinge90 jinge90 requested a review from againull December 11, 2025 04:41
@bader
Copy link
Contributor

bader commented Dec 11, 2025

We have declared __imf_fsigm* in https://github.com/intel/llvm/pull/20873/files#diff-8da594ced8852cd2e62e9d1fcd013ca0b2256b88b3745b30a239bb01aa7b2011R251
So, the declarations at the begining is redundant, just remove them.

I suggest we keep the first declaration and remove the second instead.
It keeps sigmoid declarations aligned with the rest of the declarations.

@jinge90 jinge90 marked this pull request as draft December 11, 2025 05:20
@jinge90 jinge90 changed the title [SYCL][NFC] Remove redundant __imf_fsigm* declarations [SYCL][NFC] Clean up __imf_* delarations in sycl intel math header Dec 15, 2025
@jinge90 jinge90 marked this pull request as ready for review December 18, 2025 01:35
@jinge90
Copy link
Contributor Author

jinge90 commented Dec 18, 2025

Hi, @bader
I cleaned up sycl/builtins.hpp to remove all duplicate imf function declarations and also aligned the declaration style in imf headers with numeric team's.
Could you help review?
Thanks very much.

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

@jinge90, thanks. Nice clean-up.
LGTM with a couple of suggestions.

Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 <[email protected]>
@jinge90
Copy link
Contributor Author

jinge90 commented Dec 18, 2025

Hi, @againull
Could you help review this PR?
Thanks very much.

@againull againull merged commit a12ac16 into intel:sycl Dec 18, 2025
32 of 33 checks passed
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