Skip to content

[CIR][CIRGen][Builtin][X86] Add support for tzcnt_u16, tzcnt_u32, and tzcnt_u64 #1691

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 3 commits into from
Jun 23, 2025

Conversation

ayokunle321
Copy link
Contributor

Related: #1404

Implements codegen for the X86 builtins tzcnt_u16, tzcnt_u32, and tzcnt_u64.

While adding tests for both the Intel and AMD variants of BMI intrinsics, I ran into issues when placing them in the same file.

Both _tzcnt_u16 (Intel) and __tzcnt_u16(AMD) map to the same inline wrapper in <immintrin.h>. Whether they're isolated or both are present in a test file, Clang emits only one definition (__tzcnt_u16) which I think causes FileCheck mismatches i.e., the CHECK lines for the Intel version (test_tzcnt_u16) would fail when looking for _tzcnt_u16.

I tried updating the CHECK lines for the Intel test to match the emitted symbol (__tzcnt_u16), but it still failed unless the Intel test was run in isolation, and only when CHECK was updated to _tzcnt_u16 even though __tzcnt_u16 is what is emitted. I also experimented with split-file to isolate the tests, but that didn’t resolve the issue either.

To keep the tests independent, I split the Intel and AMD tests into separate files. Was wondering if this was fine as in OG clang, both Intel and AMD variants are in the same file (https://github.com/llvm/clangir/blob/main/clang/test/CodeGen/X86/bmi-builtins.c)

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM pending test observation

@bcardosolopes bcardosolopes merged commit f74a380 into llvm:main Jun 23, 2025
9 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.

2 participants