Skip to content

[libclc] Move log/log2/log10 to CLC library #128540

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
Feb 25, 2025

Conversation

frasercrmck
Copy link
Contributor

@frasercrmck frasercrmck commented Feb 24, 2025

This commit also enables fp16 log, which was previously missing.

Other than that, no changes to codegen for AMDGPU/Nvidia targets.

Note that for simplicity this commit doesn't try to refactor or optimize the implementations. Notably, each log is only implementated for scalar types; vector types are scalarized. It doesn't look too difficult to make the implementations suitable for vector codegen, so I'll try that in a future commit.

There's also an unused implementation of log in clc_log_base.h, whereas the implementation currently used by libclc targets re-uses log2 with an additional multiplication. That should also be cleaned up as on first inspection it looks a more optimal implementation, though it would have to be checked against the OpenCL CTS for good measure.

No changes to codegen for AMDGPU/Nvidia targets.

Note that for simplicity this commit doesn't try to refactor or optimize
the implementations. Notably, each log is only implementated for scalar
types; vector types are scalarized. It doesn't look too difficult to
make the implementations suitable for vector codegen, so I'll try that
in a future commit.

There's also an unused implementation of log in clc_log_base.h, whereas
the implementation currently used by libclc targets re-uses log2 with an
additional multiplication. That should also be cleaned up as on first
inspection it looks a more optimal implementation, though it would have
to be checked against the OpenCL CTS for good measure.
@frasercrmck frasercrmck added the libclc libclc OpenCL library label Feb 24, 2025
@frasercrmck frasercrmck requested a review from arsenm February 24, 2025 17:36
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

amdgpu should directly use llvm.log* and llvm.exp* for the f16 and f32 cases, still relying on the libraries for f64

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Cleanups for another PR

@frasercrmck frasercrmck merged commit f8948d3 into llvm:main Feb 25, 2025
11 checks passed
@frasercrmck frasercrmck deleted the libclc-clc-log-log2-log10 branch February 25, 2025 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libclc libclc OpenCL library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants