Skip to content

SWDEV-548892 - Stop using ocml exp10 functions#2226

Open
arsenm wants to merge 1 commit intodevelopfrom
arsenm/swdev548892/stop-using-ocml-exp10-functions
Open

SWDEV-548892 - Stop using ocml exp10 functions#2226
arsenm wants to merge 1 commit intodevelopfrom
arsenm/swdev548892/stop-using-ocml-exp10-functions

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Dec 8, 2025

Follow exp and exp2. I only separated exp10 just in case it breaks a build somewhere, since the elementwise exp10 builtin is much more recent than the others.

@arsenm arsenm requested a review from a team as a code owner December 8, 2025 20:21
Copilot AI review requested due to automatic review settings December 8, 2025 20:21
@arsenm arsenm requested a review from a team as a code owner December 8, 2025 20:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces __ocml_exp10 function calls with the newer __builtin_elementwise_exp10 builtin for half-precision (__half, __half2) and bfloat16 (__hip_bfloat16) types, following the pattern previously established for exp and exp2 functions.

Key Changes:

  • Replaced __ocml_exp10_f16 with __builtin_elementwise_exp10 for __half type
  • Replaced __ocml_exp10_2f16 with __builtin_elementwise_exp10 for __half2 type
  • Replaced __ocml_exp10_f32 with __builtin_elementwise_exp10 for __hip_bfloat16 type

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
projects/clr/hipamd/include/hip/amd_detail/amd_hip_fp16.h Updated hexp10 and h2exp10 functions to use builtin exp10 implementation
projects/clr/hipamd/include/hip/amd_detail/amd_hip_bf16.h Updated hexp10 function for bfloat16 to use builtin exp10 implementation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fabiomestre
Copy link
Contributor

fabiomestre commented Dec 9, 2025

Given that the __builtin implementation for exp10 functions with f16 types was wrong until very recently, doesn't this change have the potential to break accuracy tests when using a slightly older compiler?

Note that currently, some of the HIP tests for Float16 accuracy are disabled. So this PR will probably still pass CI.

@arsenm arsenm force-pushed the arsenm/swdev548892/stop-using-ocml-exp10-functions branch 2 times, most recently from bc13654 to cb9b63a Compare February 3, 2026 17:23
Follow exp and exp2. I only separated exp10 just in case
it breaks a build somewhere, since the elementwise exp10 builtin
is much more recent than the others.
@arsenm arsenm force-pushed the arsenm/swdev548892/stop-using-ocml-exp10-functions branch from cb9b63a to ec9b12a Compare March 21, 2026 16:46
@arsenm
Copy link
Contributor Author

arsenm commented Mar 21, 2026

Given that the __builtin implementation for exp10 functions with f16 types was wrong until very recently, doesn't this change have the potential to break accuracy tests when using a slightly older compiler?

I would expect new headers to not work with old compilers in the general case. We don't properly version anything, so we can't detect any cases like this.

It's been over 3 months since the compiler patch now

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants