Optimize Math.Min/Max byte-overloads

[deeprobin]

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKBuIGYACMxgYUYG8bGfmngAnhhiMAsgEsAdgApBwxtjSM5I4AEpO3XtuIB2MdgwALAHQSZi5WoDcWngF87jJw2VCR5gErYA7rPcKSipWmtTaOvrS2IwAPAC8IQD8CowgyrZhvI6ZPC78AaLYCP7ylsHqoeG8egbGZkVRQTZO2dp5zORIbvKFCN5+wWUBFVw54TVRjAB8CRXJ0WnAGdrZ9kA=

The raw implementation achieves a smaller codegen than the Math.Min/Max methods.

AggresiveInlining and AggresiveOptimization do not change this fact.

Min codegen

C.Min(Byte, Byte)
    L0000: movzx eax, dl
    L0003: movzx edx, r8b
    L0007: cmp eax, edx
    L0009: jle short L000d
    L000b: jmp short L000f
    L000d: mov edx, eax
    L000f: mov eax, edx
    L0011: ret

C.MinRaw(Byte, Byte)
    L0000: movzx eax, dl
    L0003: movzx edx, r8b
    L0007: cmp eax, edx
    L0009: jle short L000e
    L000b: mov eax, edx
    L000d: ret
    L000e: ret

Diff

  movzx eax, dl
  movzx edx, r8b
  cmp eax, edx
- jle short L000d
- jmp short L000f
- mov edx, eax
+ jle short L000e
  mov eax, edx
  ret
+ ret

Max codegen

C.Max(Byte, Byte)
    L0000: movzx eax, dl
    L0003: movzx ecx, r8b
    L0007: cmp eax, ecx
    L0009: jl short L000f
    L000b: movzx eax, dl
    L000e: ret
    L000f: movzx eax, r8b
    L0013: jmp short L000e

C.MaxRaw(Byte, Byte)
    L0000: movzx eax, cl
    L0003: movzx edx, dl
    L0006: cmp eax, edx
    L0008: jge short L000d
    L000a: mov eax, edx
    L000c: ret
    L000d: ret

Diff

- movzx eax, dl
- movzx ecx, r8b
- cmp eax, ecx
- jl short L000f
- movzx eax, dl
+ movzx eax, cl
+ movzx edx, dl
+ cmp eax, edx
+ jge short L000d
+ mov eax, edx
+ ret
  ret
- movzx eax, r8b
- jmp short L000e

category:cq
theme:basic-cq

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

---

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKBuIGYACMxgYUYG8bGfmngAnhhiMAsgEsAdgApBwxtjSM5I4AEpO3XtuIB2MdgwALAHQSZi5WoDcWngF87jJw2VCR5gErYA7rPcKSipWmtTaOvrS2IwAPAC8IQD8CowgyrZhvI6ZPC78AaLYCP7ylsHqoeG8egbGZkVRQTZO2dp5zORIbvKFCN5+wWUBFVw54TVRjAB8CRXJ0WnAGdrZ9kA=

The raw implementation achieves a smaller codegen than the Math.Min/Max methods.

AggresiveInlining and AggresiveOptimization do not change this fact.

Min codegen

C.Min(Byte, Byte)
    L0000: movzx eax, dl
    L0003: movzx edx, r8b
    L0007: cmp eax, edx
    L0009: jle short L000d
    L000b: jmp short L000f
    L000d: mov edx, eax
    L000f: mov eax, edx
    L0011: ret

C.MinRaw(Byte, Byte)
    L0000: movzx eax, dl
    L0003: movzx edx, r8b
    L0007: cmp eax, edx
    L0009: jle short L000e
    L000b: mov eax, edx
    L000d: ret
    L000e: ret

Diff

  movzx eax, dl
  movzx edx, r8b
  cmp eax, edx
- jle short L000d
- jmp short L000f
- mov edx, eax
+ jle short L000e
  mov eax, edx
  ret
+ ret

Max codegen

C.Max(Byte, Byte)
    L0000: movzx eax, dl
    L0003: movzx ecx, r8b
    L0007: cmp eax, ecx
    L0009: jl short L000f
    L000b: movzx eax, dl
    L000e: ret
    L000f: movzx eax, r8b
    L0013: jmp short L000e

C.MaxRaw(Byte, Byte)
    L0000: movzx eax, cl
    L0003: movzx edx, dl
    L0006: cmp eax, edx
    L0008: jge short L000d
    L000a: mov eax, edx
    L000c: ret
    L000d: ret

Diff

- movzx eax, dl
- movzx ecx, r8b
- cmp eax, ecx
- jl short L000f
- movzx eax, dl
+ movzx eax, cl
+ movzx edx, dl
+ cmp eax, edx
+ jge short L000d
+ mov eax, edx
+ ret
  ret
- movzx eax, r8b
- jmp short L000e

Author:	deeprobin
Assignees:	-
Labels:	area-CodeGen-coreclr, untriaged
Milestone:	-

[deeprobin]

I get even better optimizations using gcc/clang.

Min

https://godbolt.org/z/T59TqoTPs

MinRaw(unsigned char, unsigned char):
        cmp     sil, dil
        mov     eax, edi
        cmovbe  eax, esi
        ret

Max

https://godbolt.org/z/jb6K664bx

MaxRaw(unsigned char, unsigned char):
        cmp     sil, dil
        mov     eax, edi
        cmovnb  eax, esi
        ret

@echesakovMSFT I heard you are working on cmov. Maybe this issue is interesting for you.

[tannergooding]

The raw implementation achieves a smaller codegen than the Math.Min/Max methods.

Inlining can impact branches in some weird ways (CC. @AndyAyersMS)

I get even better optimizations using gcc/clang.

Smaller codegen doesn't necessarily equate to better. cmov has some considerations that make it not always a good choice and while modern hardware has gotten better here, the recommendation from the Architecture Optimization manuals is still that its only used when the odds are approx 50/50.

This makes it a good default for things like Max/Min; but it should also, ideally, be influenced by things like Dynamic PGO so that if one path becomes more likely; cmov is no longer used.

[AndyAyersMS]

Thanks for the heads up, there is still work we hope to get to 7.0 that might help here.

Oddly enough we (at least in main) have PGO data for Max that shows our usage of it is biased (based on ~600K calls). Looks like 6.0 data may be similar which explains that backwards jump. The jit is a bit too aggressive in rearranging blocks when it has PGO data. A 67/33 split is not sufficient evidence to force the less likely path into two jumps.

Trees after Morph - Inlining

-----------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd                 weight   IBC  lp [IL range]     [jump]      [EH region]         [flags]
-----------------------------------------------------------------------------------------------------------------------------------------
BB01 [0000]  1                             1           [000..008)                                     i 
BB02 [0001]  1                             1    100    [000..001)-> BB04 ( cond )                     i IBC 
BB03 [0002]  1                             0.33  33    [000..001)-> BB05 (always)                     i IBC 
BB04 [0003]  1                             0.67  67    [000..001)                                     i IBC 
BB05 [0004]  1                             1    100    [???..???)        (return)                     internal IBC 
-----------------------------------------------------------------------------------------------------------------------------------------

[JulieLeeMSFT]

there is still work we hope to get to 7.0 that might help here.

Opportunistically setting this to 7.0 to see if other work will improve this issus.

[AndyAyersMS]

Don't see us getting to this in 7.0, so moving to future.

[AndyAyersMS]

Note

AI-assisted (Copilot CLI).

Verified resolved on current main. All four variants now produce identical 14-byte codegen on Linux x64, using cmov instead of a branch.

Linux x64 codegen on current main (crossgen2)

Min(byte, byte) = Math.Min(a, b):

movzx rax, dil
movzx rcx, sil
cmp   eax, ecx
cmovg eax, ecx     ; keep smaller
ret

MinRaw(byte, byte) = a < b ? a : b:

movzx rax, dil
movzx rcx, sil
cmp   eax, ecx
cmovge eax, ecx    ; keep smaller (different cond, same effect)
ret

Max(byte, byte) = Math.Max(a, b):

movzx rax, dil
movzx rcx, sil
cmp   eax, ecx
cmovl eax, ecx     ; keep greater
ret

MaxRaw(byte, byte) = a > b ? a : b:

movzx rax, dil
movzx rcx, sil
cmp   eax, ecx
cmovle eax, ecx    ; keep greater (different cond, same effect)
ret

All four methods: 14 bytes / 5 instructions, branch-free. The 2022 codegen (which had cmp + jl + jmp + mov chains in Min/Max but cmov-equivalent in the Raw variants) is gone. The "PGO too aggressive in rearranging blocks" issue Andy diagnosed in 2022 is no longer visible here.

Suggest closing as resolved.