Fix ILogB for Half and BFloat16 subnormal values#123294
Merged
stephentoub merged 8 commits intomainfrom Jan 20, 2026
Merged
Conversation
BitOperations.LeadingZeroCount has no ushort overload, so the TrailingSignificand is implicitly widened to uint, adding 16 extra leading zeros. Subtract 16 to correct for this widening. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix possibly incorrect implementation of ILogB for BFloat16 and Half
Fix ILogB for Half and BFloat16 subnormal values
Jan 17, 2026
stephentoub
reviewed
Jan 17, 2026
…byte Instead of adjusting the value at the call site, add internal overloads of BitOperations.LeadingZeroCount for ushort and byte types that handle the widening adjustment internally. Update UInt16, Int16, Char, Byte, and SByte LeadingZeroCount methods to use these new internal overloads. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
stephentoub
approved these changes
Jan 17, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in ILogB for Half and BFloat16 where subnormal values returned incorrect results due to implicit type widening. When BitOperations.LeadingZeroCount was called with a ushort argument (the type of TrailingSignificand), it was implicitly widened to uint, adding 16 extra leading zeros to the count.
Changes:
- Added internal
LeadingZeroCountoverloads forushortandbyteinBitOperationsthat handle the bit-width adjustment - Refactored
LeadingZeroCountinUInt16,Int16,Byte,SByte, andCharto use the new internal overloads, removing redundant manual adjustments - Added comprehensive tests for
ILogBcovering normal, special, and subnormal values for bothHalfandBFloat16
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| BitOperations.cs | Adds internal LeadingZeroCount overloads for ushort and byte that correctly adjust for bit width when widening to uint |
| UInt16.cs | Removes manual -16 adjustment, now uses internal LeadingZeroCount(ushort) overload |
| Int16.cs | Removes manual -16 adjustment, now uses internal LeadingZeroCount(ushort) overload |
| Byte.cs | Removes manual -24 adjustment, now uses internal LeadingZeroCount(byte) overload |
| SByte.cs | Removes manual -24 adjustment, now uses internal LeadingZeroCount(byte) overload |
| Char.cs | Removes manual -16 adjustment, now uses internal LeadingZeroCount(ushort) overload |
| HalfTests.cs | Adds tests for ILogB including normal values, special values, and two subnormal test cases |
| BFloat16Tests.cs | Adds tests for ILogB including normal values, special values, and two subnormal test cases |
Per reviewer feedback, use the existing ushort.LeadingZeroCount method which already handles the bit-width adjustment, instead of adding new internal overloads to BitOperations. Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
src/libraries/System.Private.CoreLib/src/System/Numerics/BFloat16.cs
Outdated
Show resolved
Hide resolved
Merged latest changes from main and updated BFloat16.ILogB to use ushort.LeadingZeroCount consistently with Half.ILogB, since TrailingSignificand is ushort for both types. Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
Copilot
AI
requested review from
MichalStrehovsky,
steveisok and
vitek-karas
as code owners
January 19, 2026 18:17
Undoes the previous merge commit that was not done correctly. Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
…ormula - Changed TrailingSignificand property from ushort to byte - Changed ExtractTrailingSignificandFromBits to return byte - Changed MinTrailingSignificand and MaxTrailingSignificand constants to byte - Updated IsPow2 to use byte.PopCount - Updated ILogB to use byte.LeadingZeroCount with adjusted formula Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
tannergooding
approved these changes
Jan 19, 2026
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.
ILogBforHalfandBFloat16returns incorrect values for subnormal numbers becauseBitOperations.LeadingZeroCountonly has overloads foruint/ulong/nuint, so theushortfromTrailingSignificandis implicitly widened touint, adding 16 extra leading zerosHalf.ILogBto useushort.LeadingZeroCountwhich handles the bit-width adjustmentBFloat16.ILogBto usebyte.LeadingZeroCountafter changingTrailingSignificandto returnbyteHalf.ILogBandBFloat16.ILogBwith normal and subnormal valuesSecurity Summary
No security vulnerabilities were discovered or introduced by these changes.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.