Skip to content

[CIR][CIRGen][Builtin][X86] Lower lzcnt_u16, lzcnt_u32, lzcnt_u64 #1683

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RiverDave
Copy link

@RiverDave RiverDave commented Jun 14, 2025

No test cases provided for lzcnt_u16 as presented in the OG codegen equivalent: test/CodeGen/X86/lzcnt-builtins.c.

related: #1404

@RiverDave RiverDave force-pushed the lower_lzcntu64/u32 branch from f6ba347 to b460fb3 Compare June 14, 2025 02:48
@RiverDave RiverDave changed the title [CIR][CIRGen][Builtin][X86] Lower lzcnt_u32 and lzcnt_u64 [CIR][CIRGen][Builtin][X86] Lower lzcnt_u16, lzcnt_u32, lzcnt_u64 Jun 14, 2025
@ayokunle321
Copy link
Contributor

Hey @RiverDave! Wanted to help out with the X86 builtins so started working on tzcnt. For some odd reason when I test it, Clang is making a helper function (first function) for the intrinsic and then making a call (in the second function) to the helper. Its kinda funny but yeah I was wondering if you've seen that before or could help out.
Screenshot 2025-06-14 at 7 12 12 PM

@RiverDave
Copy link
Author

RiverDave commented Jun 15, 2025

Hey @RiverDave! Wanted to help out with the X86 builtins so started working on tzcnt. For some odd reason when I test it, Clang is making a helper function (first function) for the intrinsic and then making a call (in the second function) to the helper. Its kinda funny but yeah I was wondering if you've seen that before or could help out. Screenshot 2025-06-14 at 7 12 12 PM

Hi, Not an expert here (only started working on cir a couple of days ago), but I believe that's expected.

What I understand is that we first lower the test function that makes a call to your intrinsic (we don't really care much about those in test cases). Later based on the intrinsic id parsed it generates a fn definition __lzcnt_16 in your case, that's where you're lowering implementation comes in place, while I'm not sure why this behaviour is different amongst other intrinsics, I'd only make sure your intrinsic op is called in the "helper" which seems to be happening based on the sc shared.

It might be better to ask in the discord channel to be 100% sure.

@ayokunle321
Copy link
Contributor

Okay, no problem. Thanks! I've asked on the Discord.

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.

Great, thanks! One minor change needed and good to go

@RiverDave RiverDave force-pushed the lower_lzcntu64/u32 branch from b460fb3 to 0877aa6 Compare June 18, 2025 01:53
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.

3 participants