[JIT] Could Scalable statistics implementation avoid BSR?

[sfiruch]

I noticed the Scalable statistics counters of the JIT in a profile today. @AndyAyersMS have written a very helpful documentation about the counters here: https://github.com/dotnet/runtime/blob/main/docs/design/features/ScalableApproximateCounting.md

This is the actual code from the JIT:

runtime/src/coreclr/vm/jithelpers.cpp

Lines 3693 to 3721 in 35e23f7

	HCIMPL1(void, JIT_CountProfile32, volatile LONG* pCounter)
	{
	FCALL_CONTRACT;
	FC_GC_POLL_NOT_NEEDED();
	LONG count = *pCounter;
	LONG delta = 1;
	DWORD threshold = g_pConfig->TieredPGO_ScalableCountThreshold();
	if (count > 0)
	{
	DWORD logCount = 0;
	BitScanReverse(&logCount, count);
	if (logCount >= threshold)
	{
	delta = 1 << (logCount - (threshold - 1));
	const unsigned rand = HandleHistogramProfileRand();
	const bool update = (rand & (delta - 1)) == 0;
	if (!update)
	{
	return;
	}
	}
	}
	InterlockedAdd(pCounter, delta);
	}
	HCIMPLEND

You can see that it first computes the log of the counter using BSR on line 3705. This is then compared to the threshold on L3707, which then leads us (or the CPU) down the approximate counting path.

My question is: Why does the code compare the log of the counter with the threshold, and not the counter directly? IMHO the code could've been written this way, eliminating the BSR for the non-approximate case:

 HCIMPL1(void, JIT_CountProfile32, volatile LONG* pCounter) 
 { 
     FCALL_CONTRACT; 
     FC_GC_POLL_NOT_NEEDED(); 
  
     LONG count = *pCounter; 
     LONG delta = 1; 
     DWORD threshold = g_pConfig->TieredPGO_ScalableCountThreshold(); 
  
     if (count > 0) 
     { 
-        DWORD logCount = 0; 
-        BitScanReverse(&logCount, count); 
  
-        if (logCount >= threshold) 
+        if (count >= threshold) 
         { 
+            DWORD logCount = 0; 
+            BitScanReverse(&logCount, count); 

             delta = 1 << (logCount - (threshold - 1)); 
             const unsigned rand = HandleHistogramProfileRand(); 
             const bool update = (rand & (delta - 1)) == 0; 
             if (!update) 
             { 
                 return; 
             } 
         } 
     } 
  
     InterlockedAdd(pCounter, delta); 
 } 
 HCIMPLEND 

[sfiruch]

Answer: Yes, the BSR can be avoided. My PR (#110258) improving this was merged for .NET 10.