Repository navigation
Make _sincos reach libm's sincos on every Linux libc, and use it in cagnoli - #14
Merged
Merged
Conversation
…in `cagnoli` `_sincos` used `__builtin_sincos`, which Clang turns into the `llvm.sincos` intrinsic. LLVM lowers that intrinsic to a `sincos` call only for the C libraries it knows to have one (glibc, Android, Fuchsia) and splits it back into separate `sin` and `cos` calls for the others, musl among them, so on a musl build the helper saved nothing. A plain call to a function named `sincos` is recognized and lowered the same way. Declare the function under another C name with the `sincos` assembler label instead: the compiler does not recognize it, and the call reaches libm as written under Clang, GCC and ThinLTO. Every Linux libc provides `sincos`; other platforms keep the builtin. `cagnoli` (the cell area functions) still took `sin` and `cos` of three angles separately; with the helper fixed, use it there too. `h3CellAreaM2` in ClickHouse was 20% slower on aarch64 with musl because of these calls. All 316 tests pass; the results are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The previous commit picked up `clang-format` output that the build applied in place to files this change does not touch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thevar1able
added a commit
to ClickHouse/ClickHouse
that referenced
this pull request
Oct 21, 2026
ClickHouse/h3#14 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_sincos(added in 3d5dfad) used__builtin_sincos, which Clang turns into thellvm.sincosintrinsic. LLVM lowers that intrinsic to asincoscall only for the C libraries it knows to have one (glibc, Android, Fuchsia) and splits it back into separatesinandcoscalls for the others, musl among them, so on a musl build the helper saved nothing. A plain call to a function namedsincosis recognized and lowered the same way.Declare the function under another C name with the
sincosassembler label instead: the compiler does not recognize it, and the call reaches libm as written. Verified onx86_64-linux-muslwith Clang, with ThinLTO, and with GCC. Every Linux libc providessincos; other platforms keep the builtin.cagnoli(the cell area functions) still tooksinandcosof three angles separately; with the helper fixed, use it there too.h3CellAreaM2in ClickHouse was 20% slower on aarch64 with musl because of these calls (performance report: https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=109239&sha=a685010407d544ee0b02294d34118bd48f46ff84&name_0=PR&name_1=Performance%20Comparison%20%28arm_release%2C%20master_head%2C%201%2F6%29).All 316 tests pass; the results are unchanged.
Related: ClickHouse/ClickHouse#109239
🤖 Generated with Claude Code