Skip to content

[libclc] Move several integer functions to CLC library #116786

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 1 commit into from
Jan 29, 2025

Conversation

frasercrmck
Copy link
Contributor

@frasercrmck frasercrmck commented Nov 19, 2024

This commit moves over the OpenCL clz, hadd, mad24, mad_hi, mul24, mul_hi, popcount, rhadd, and upsample builtins to the CLC library.

This commit also optimizes the vector forms of the mul_hi and upsample builtins to consistently remain in vector types, instead of recursively splitting vectors down to the scalar form.

The OpenCL mad_hi builtin wasn't previously publicly available from the CLC libraries, as it was hash-defined to mul_hi in the header files. That issue has been fixed, and mad_hi is now exposed.

The custom AMD implementation/workaround for popcount has been removed as it was only required for clang < 7.

There are still two integer functions which haven't been moved over. The OpenCL mad_sat builtin uses many of the other integer builtins, and would benefit from optimization for vector types. That can take place in a follow-up commit. The rotate builtin could similarly use some more dedicated focus, potentially using clang builtins.

@frasercrmck frasercrmck added the libclc libclc OpenCL library label Nov 19, 2024
@frasercrmck frasercrmck requested a review from arsenm November 19, 2024 11:25
return x ? __builtin_clzl(x) : 64;
}

_CLC_UNARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, char, __clc_clz, char)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be nice if __builtin_clz* builtins could operate on vector types. I'll make a note of this improvement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That might be better and less ambiguous as a new __builtin_elementwise_clz builtin, come to think of it.

@frasercrmck
Copy link
Contributor Author

@arsenm are you happy for this to be merged?

@frasercrmck
Copy link
Contributor Author

ping

@frasercrmck
Copy link
Contributor Author

The updated version of this patch removes the SPIR-V header workarounds, and optimizes the upsample and mul_hi implementations for vector types. All the integer functions in this patch now avoid scalarization.

This commit moves over the OpenCL clz, hadd, mad24, mad_hi, mul24,
mul_hi, popcount, rhadd, and upsample builtins to the CLC library. There
are no changes to any target's CLC libraries.

The OpenCL mad_hi builtin wasn't previously publicly available from the
CLC libraries, as it was hash-defined to mul_hi in the header files.
That issue has been fixed, and mad_hi is now exposed.

The custom AMD implementation/workaround for popcount has been removed
as it was only valid for clang < 7.

There are still two integer functions which haven't been moved over. The
OpenCL add_sat, sub_sat, and mad_sat builtins require saturating
conversion builtins which haven't yet been ported.
@frasercrmck frasercrmck merged commit 7441e87 into llvm:main Jan 29, 2025
8 checks passed
@frasercrmck frasercrmck deleted the libclc-clc-integer branch January 29, 2025 13: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.

4 participants