- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.2k
Move arithmetic helpers to managed code #109087
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
Conversation
ac4bd28    to
    3c98d26      
    Compare
  
            
          
                src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      b6f3e2b    to
    8b0448f      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check the perf impact on the affected architectures to see whether it is something we can live with.
There is alternatives with better perf possible: Inline the exception throwing in the JITed code (and optimize it out if the JIT can prove that it cannot happen). We do that on arm64 today.
d7c87f8    to
    f340359      
    Compare
  
    2e344a5    to
    c2bf9cc      
    Compare
  
    c2bf9cc    to
    2a86f4d      
    Compare
  
    | @AaronRobinsonMSFT, win-x86 leg is failing this assert: Does it mean we need to use something like  | 
| 
 
 I think the crash that you are seeing is caused by calling convention mismatch for 64-bit helpers. The existing helpers use  The existing NAOT implementation deals with the calling convention issues with using QCalls that use standard calling convention instead of managed calling convention. | 
| Software fallback impl was much worse: diff to base ratio is 61.50 vs. 1.58 with the FCall. | 
| @EgorBot -windows_intel -use32bit using BenchmarkDotNet.Attributes;
public class Bench
{
    long a = 10000000000000000L;
    long b = 1000L;
    [Benchmark]
    public long SDiv() => a / b;
} | 
This reverts commit 9dbcf4e.
| 
 With QCall+COMPlusThrow approach (f0cd111), it's 2.91. 🥲 | 
        
          
                src/libraries/System.Private.CoreLib/src/System/Math.DivModInt.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | This is blocked until that half nanoseconds regression on x86 is sorted out on JIT side. Lets revisit this when that happens. | 
Only the FCThrow ones in jithelpers have been moved.
The(JIT_GetRefAny is handled in #110344)JIT_GetRefAnymethod was left out due to the existing TODO regarding type inheritance.