Update new ArgumentOutOfRangeException.Throw methods to include ActualValue

[stephentoub]

No description provided.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	stephentoub
Assignees:	stephentoub
Labels:	area-System.Runtime
Milestone:	-

[dakersnar]

Are we including value in the exception purely for debug purposes, or would a try/catch statement be able to do something with value when the exception is caught?

[stephentoub]

Are we including value in the exception purely for debug purposes, or would a try/catch statement be able to do something with value when the exception is caught?

Primarily diagnostics; the value will show up in the Exception.Message. But it's also possible for a catch block to access the ArgumentOutOfRangeException.ActualValue if there's a need.

[AndyAyersMS]

@stephentoub how hard would it be to avoid permuting the arguments in cases like these?

For example

https://github.com/dotnet/runtime/blame/68b410f5b293a4cfa81f3db3a173c3459d5dd9de/src/libraries/System.Private.CoreLib/src/System/ArgumentOutOfRangeException.cs#L195-L204

The jit is not very smart about handling the constraints this creates and ends up doing extra work in the frequent case instead of deferring it all to the throw case:

; Assembly listing for method System.ArgumentOutOfRangeException:ThrowIfGreaterThan[ulong](ulong,ulong,System.String) (Tier1)
; Emitting BLENDED_CODE for X64 with AVX512 - Windows
; Tier1 code
; optimized code
; rsp based frame
; partially interruptible
; No matching PGO data
; 0 inlinees with PGO data; 0 single block inlinees; 1 inlinees without PGO data
; Final local variable assignments
;
;  V00 arg0         [V00,T00] (  5,  3.50)    long  ->  rax         ld-addr-op single-def
;  V01 arg1         [V01,T01] (  5,  3.50)    long  ->  r10         single-def
;  V02 arg2         [V02,T02] (  3,  2   )     ref  ->   r8         class-hnd single-def
;  V03 OutArgs      [V03    ] (  1,  1   )  struct (32) [rsp+00H]   do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V04 tmp1         [V04,T03] (  0,  0   )     int  ->  zero-ref    "Inline return value spill temp"
;
; Lcl frame size = 40

G_M37737_IG01:        ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0000 {}, byref, nogc <-- Prolog IG
       sub      rsp, 40
       mov      rax, rcx
       mov      r10, rdx
						;; size=10 bbWeight=1 PerfScore 0.75
G_M37737_IG02:        ; bbWeight=1, gcrefRegs=0100 {r8}, byrefRegs=0000 {}, byref, isz
       ; gcrRegs +[r8]
       cmp      rax, r10
       jb       SHORT G_M37737_IG04
						;; size=5 bbWeight=1 PerfScore 1.25
G_M37737_IG03:        ; bbWeight=0.50, gcrefRegs=0100 {r8}, byrefRegs=0000 {}, byref, isz
       cmp      rax, r10
       ja       SHORT G_M37737_IG05
						;; size=5 bbWeight=0.50 PerfScore 0.62
G_M37737_IG04:        ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0000 {}, byref, epilog, nogc
       ; gcrRegs -[r8]
       add      rsp, 40
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25
G_M37737_IG05:        ; bbWeight=0, gcVars=0000000000000000 {}, gcrefRegs=0100 {r8}, byrefRegs=0000 {}, gcvars, byref
       ; gcrRegs +[r8]
       mov      rcx, r8
       ; gcrRegs +[rcx]
       mov      rdx, rax
       mov      r8, r10
       ; gcrRegs -[r8]
       call     [System.ArgumentOutOfRangeException:ThrowGreater[ulong](System.String,ulong,ulong)]
       ; gcrRegs -[rcx]
       ; gcr arg pop 0
       int3   

I suppose it may not matter much if typically these methods get inlined -- then the ABI constraints are lifted.

[stephentoub]

I suppose it may not matter much if typically these methods get inlined

The goal is for this method always be inlined; if it's not, that's a bigger problem.

What would you recommend? If including the value in the exception is problematic, we can do without it... having it is just a nice boon for understanding what went wrong.

[AndyAyersMS]

What would you recommend?

If possible, pass the parameters down in the same order that they're passed in (same goes for most any "wrapper" style method)

public static void ThrowIfLessThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
    where T : IComparable<T>
{
    if (value.CompareTo(other) <= 0)
        // ThrowLessEqual(paramName, value, other);
        ThrowLessEqual(value, other, paramName);
}

Even if these helpers are always inlined in jitted code, doing this might give a small code size reduction in R2R.