Skip to content
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

Calling the llvmint::cttz_iX is broken #34382

Closed
gnzlbg opened this issue Jun 20, 2016 · 8 comments
Closed

Calling the llvmint::cttz_iX is broken #34382

gnzlbg opened this issue Jun 20, 2016 · 8 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 20, 2016

I am trying to call the llvmint::cttz_i8(1i8, false) intrinsic from the llvmint crate and am getting the following error:

is_zero_undef argument of bit counting intrinsics must be a constant int
 %17 = call i8 @llvm.cttz.i8(i8 %14, i1 zeroext %16)

The problem seems to be that rustc is not propagating false to LLVM properly.

This was also reported to the llvmint crate: huonw/llvmint#5

@nagisa
Copy link
Member

nagisa commented Jun 20, 2016

Did you find trailing_zeros method on integers to be too stable? There’s always the std::intrinsics module which contains proper implementation of the intrinsic. I doubt this will ever be fixed, primarily because calling llvm intrinsics will never be stable and there’s no reason to fix it.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 20, 2016

Is the handling of zero by that intrinsic defined or undefined? The documentation does not say, and one might want to do both (hence why llvmint::cttz_iX takes two parameters).

@nagisa
Copy link
Member

nagisa commented Jun 20, 2016

I think the best correct solution you can hope for here is new intrinsics in std::intrinsics which could be added provided there’s a strong usecase for zero-is-undef (performance is unlikely to be one).

re zero: answered on the other issue: #34381.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 20, 2016

I think the best correct solution you can hope for here is new intrinsics in std::intrinsics which could be added provided there’s a strong usecase for zero-is-undef (performance is unlikely to be one).

So this turned into an XY Problem.

What I actually wanted was to use the BMI 1.0 tzcnt intrinsic without writting any assembly.

Ideally, std::intrinsics::cttz would use this if the architecture supports BMI 1.0. LLVM will use that instruction automatically if available when calling through the llvm.cttz.iX intrinsic (and will skip the zero check if it can prove that the value cannot be zero), but since there is no architecture flag for BMI 1.0 I don't know how to tell LLVM via rustc that the architecture supports it (and I don't know if the LLVM version used by rustc implements it, but it is new enough that I would guess it does).

So to work around the lack of a feature flag I ended up adding my own feature flags to my crate and trying to call llvm intrinsics for the particular architectures directly and running into this.

@eefriedman
Copy link
Contributor

-C target-feature=+bmi

@steveklabnik steveklabnik added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Jun 20, 2016
@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 21, 2016

-C target-feature=+bmi

@eefriedman Thanks! Are those documented somewhere (i've tried googling for the supported target-features without any luck)? Is that for BMI 1.0 or BMI 2.0?

EDIT: found it! The command llc -mattr=help can be used to list all target features!

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 21, 2016

Is there a way to detect the target-feature's enabled from within a rust program? Something like #[cfg(target_feature = "bmi")] that is true if that target feature is enabled?

gnzlbg added a commit to gnzlbg/rust that referenced this issue Jun 21, 2016
This commit adds support for detecting all of llc's target features for the x86/x86_64 architectures.

Closes rust-lang#30462.
Helps with rust-lang#29717 and rust-lang#34382.
@Mark-Simulacrum
Copy link
Member

I suspect that this is no longer an issue judging by the comments, with the proper solution being target-feature=+bmi. The original issue is likely impossible to reproduce today since the llvmint crate has not received updates in 2 years. As such, I'm going to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
Projects
None yet
Development

No branches or pull requests

5 participants