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

Documentation: std::intrinsics::cttz does not specify the handling of zero. #34381

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

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 20, 2016

The llvm docs for `llvm.cttz.iX(value, flag) specifies that a flag can be passed to control whether zero provides a defined result or not.

Whether std::intrinsics::cttz provides a defined result for zero or not should be specified in the documentation, since if not some code might require if x == 0 { mem::sizeof(x) * 8 } else { std::intrinsics::cttz(x) } to be used (instead of just std::intrinsics::cttz(x)).

See the LLVM documentation: http://llvm.org/docs/LangRef.html#llvm-cttz-intrinsic

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 20, 2016

From:

"ctpop" | "ctlz" | "cttz" | "bswap" => (1, vec!(param(ccx, 0)), param(ccx, 0)),

It might seem that the intrinsic is always called with a value of 0 (false) but I cannot find where the std::intrinsic::cttz function is implemented to verify this. And the LLVM docs are a bit confusing about what exactly the flag means. The text seems to indicate that true means "consistent handling of zero" but the argument name "is_zero_undef" indicates that true means "inconsistent handling of zero".

@nagisa
Copy link
Member

nagisa commented Jun 20, 2016

Ah I see now. This intrinsic is used to implement u*::trailing_zeros method, therefore we cannot afford it to be undef for corner-case values. It seems safe to say we pass whatever the flag (which turns out to be false for “0 is defined”) makes 0 input defined for this intrinsic without even checking the implementation.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 20, 2016

This intrinsic is used to implement u*::trailing_zeros method, therefore we cannot afford it to be undef for corner-case values.

That makes sense. Having to do x == 0? 8 * CHAR_BITS : __builtin_ctz(x) in C++ is error prone (since this behavior is what is actually desired all of the time).

Where is u*::trailing_zeros / std::intrinsic::cttz implemented? I was trying to find the source to see which argument is passed without any luck.

I guess to improve the docs it would be enough to add a note like:

note: when x == 0 the behavior of std::intrinsic::cttz(x) is defined in all platforms to return 8 * mem::sizeof(x) (that is, the llvm.cttz.iX intrinsic is called with the flag zero_is_undef set to false).

@nagisa
Copy link
Member

nagisa commented Jun 20, 2016

Where is u*::trailing_zeros / std::intrinsic::cttz implemented? I was trying to find the source to see which argument is passed without any luck.

Easy way to check that is to ask rustc to emit LLVM-IR for your code. If you click LLVM IR on this playpen, you’ll find a %3 = call i64 @llvm.cttz.i64(i64 %2, i1 false), !dbg !25.

Trailing_zeros is implemented here.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 20, 2016

Trailing_zeros is implemented here.

Thanks! Where is std::intrinsic::cttz implemented? That should call/generate the llvm.cttz intrinsic and pass the arguments somehow.

If you click LLVM IR on this playpen, you’ll find a %3 = call i64 @llvm.cttz.i64(i64 %2, i1 false), !dbg !25.

Nice, TIL! One can also output the asm directly in the playpen :)

@nagisa
Copy link
Member

nagisa commented Jun 20, 2016

Where is std::intrinsic::cttz implemented?

Its implemented deep inside the translator and is not you can tweak without modifying the compiler itself. The LLVM intrinsic is emitted here and here.

@frewsxcv
Copy link
Member

PR opened #38310

frewsxcv added a commit to frewsxcv/rust that referenced this issue Dec 15, 2016
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 4, 2017
Clarify zero-value behavior of `ctlz`/`cttz` intrinsics.

Fixes rust-lang#34381.
steveklabnik added a commit to steveklabnik/rust that referenced this issue Jan 4, 2017
Clarify zero-value behavior of `ctlz`/`cttz` intrinsics.

Fixes rust-lang#34381.
bors added a commit that referenced this issue Jan 9, 2017
Clarify zero-value behavior of `ctlz`/`cttz` intrinsics.

Fixes #34381.
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

No branches or pull requests

4 participants