Skip to content

Replace FloatUtils Conversion to Integral types through unions with casting - #7676

Open
abhishekdahad-1 wants to merge 2 commits into
dotnet:mainfrom
abhishekdahad-1:HashRoundRegisterSpill
Open

Replace FloatUtils Conversion to Integral types through unions with casting#7676
abhishekdahad-1 wants to merge 2 commits into
dotnet:mainfrom
abhishekdahad-1:HashRoundRegisterSpill

Conversation

@abhishekdahad-1

Copy link
Copy Markdown

FloatUtils.cs contains an overloaded function GetBits that takes Single/Double precision floating point values and converts them into the corresponding unsigned integral types with the same width. The disassembly for this function will have a store-load dependency which can be reduced to a single mov instruction.

The function utilizes the internal union that is stored in the class, by writing into the floating point field, and then reading from the integral field. This can be observed in the Microsoft.ML.PerformanceTests.HashBench.HashScalarDouble testcase, when inspecting the HashRound function in Hashing.cs:

private uint HashRound(uint seed, double value, bool old)
{
      ulong v = FloatUtils.GetBits(value == 0 ? 0 : value);
      var hash = Hashing.MurmurRound(seed, Utils.GetLo(v));
      var hi = Utils.GetHi(v);
      if (old && hi == 0)
          return hash;
      return Hashing.MurmurRound(hash, hi);
}

When value is converted from a double to a ulong, the corresponding assembly code will look like this because of union semantics:

vmovsd qword ptr [rsp+0x28], xmm1		
mov rdx, qword ptr [rsp+0x28]		

This can be collapsed into a single instruction like so:

movq rdx, xmm1

When benchmarked on both Microsoft.ML.PerformanceTests.HashBench.HashScalarDouble and Microsoft.ML.PerformanceTests.HashBench.HashScalarFloat, it shows a performance improvement.
Before:

Method Mean Error StdDev Extra Metric
HashScalarDouble 485.0 us 9.43 us 10.09 us -
HashScalarFloat 376.4 us 0.71 us 0.63 us -

After:

Method Mean Error StdDev Extra Metric
HashScalarDouble 472.2 us 2.44 us 2.16 us -
HashScalarFloat 362.3 us 0.82 us 0.77 us -

@abhishekdahad-1
abhishekdahad-1 marked this pull request as ready for review August 24, 2026 16:46
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.88%. Comparing base (b97cca4) to head (d3247bd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7676      +/-   ##
==========================================
- Coverage   69.88%   69.88%   -0.01%     
==========================================
  Files        1487     1487              
  Lines      276240   276236       -4     
  Branches    28287    28287              
==========================================
- Hits       193046   193043       -3     
- Misses      75703    75705       +2     
+ Partials     7491     7488       -3     
Flag Coverage Δ
Debug 69.88% <100.00%> (-0.01%) ⬇️
production 64.06% <100.00%> (-0.01%) ⬇️
test 89.83% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/Microsoft.ML.Core/Utilities/FloatUtils.cs 11.24% <100.00%> (-0.85%) ⬇️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@matouskozak matouskozak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the PR, the changes looks good to me. There might be other areas where we can do similar improvements (e.g.

public static bool IsFinite(double x)
{
var bits = default(DoubleBits);
bits.Float = x;
return bits.IsFinite();
}
public static bool IsFinite(float x)
{
var bits = default(SingleBits);
bits.Float = x;
return bits.IsFinite();
}
). Let me know if you want to handle those also as part of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants