Repository navigation
typeof(T).IsValueType not completely unimpactful in some cases #48605
Copy link
Copy link
Closed
Labels
area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMICLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone
Description
The JIT has an optimization to treat typeof(T).IsValueType as a const based on the T being specified, but in some cases the presence of the check is still having an impact on the generated asm.
Non-sensical repro (derived from much more complicated real-world scenario in dotnet/roslyn#51383):
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
[DisassemblyDiagnoser]
public class Program
{
static void Main(string[] args) => BenchmarkSwitcher.FromAssemblies(new[] { typeof(Program).Assembly }).Run(args);
}
[DisassemblyDiagnoser]
public class Benchmarks
{
private Wrapper<int> _array = new Wrapper<int>(new int[1]);
private int _offset = 0;
[Benchmark]
public void Test1() => _array.Get1(_offset) = default;
[Benchmark]
public void Test2() => _array.Get2(_offset) = default;
}
public struct Wrapper<T>
{
private T[] _items;
public Wrapper(T[] item) => _items = item;
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public ref T Get1(int index)
{
return ref _items[index];
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public ref T Get2(int index)
{
if (typeof(T).IsValueType)
{
return ref _items[index];
}
return ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(_items), index);
}
}The non-value type branch in Get2 should in theory be removed completely, but using a recent .NET 6 nightly, this results in:
.NET 6.0.0 (6.0.21.11705), X64 RyuJIT
; Benchmarks.Test1()
sub rsp,28
lea rax,[rcx+10]
mov edx,[rcx+8]
mov rax,[rax]
cmp edx,[rax+8]
jae short M00_L00
movsxd rdx,edx
xor ecx,ecx
mov [rax+rdx*4+10],ecx
add rsp,28
ret
M00_L00:
call CORINFO_HELP_RNGCHKFAIL
int 3
; Total bytes of code 39.NET 6.0.0 (6.0.21.11705), X64 RyuJIT
; Benchmarks.Test2()
sub rsp,28
lea rax,[rcx+10]
mov edx,[rcx+8]
mov rax,[rax]
cmp edx,[rax+8]
jae short M00_L00
movsxd rdx,edx
lea rax,[rax+rdx*4+10]
xor edx,edx
mov [rax],edx
add rsp,28
ret
M00_L00:
call CORINFO_HELP_RNGCHKFAIL
int 3
; Total bytes of code 42Note the difference between:
xor ecx,ecx
mov [rax+rdx*4+10],ecx
and
lea rax,[rax+rdx*4+10]
xor edx,edx
mov [rax],edx
category:cq
theme:optimization
skill-level:expert
cost:large
impact:medium
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMICLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Activity
[-]typeof(T).IsValueType not completely removed in some cases[/-][+]typeof(T).IsValueType not completely unimpactful in some cases[/+]EgorBo commentedon Feb 22, 2021
Well, it's still removed, it's just some extra
lea- perhaps related to some of the recent struct improvements work?stephentoub commentedon Feb 22, 2021
Right, I meant its impact should be completely removed. But it still has an impact.
AndyAyersMS commentedon Feb 22, 2021
This is likely another example highlighting the need for forward substitution in the JIT (#6973).
The statement that gets removed also breaks up a tree and so inhibits tree-based optimizations like address mode building. We don't have anything that will try and glue broken trees back together.
JulieLeeMSFT commentedon Feb 25, 2021
@AndyAyersMS so no solution until we implement forward substitution?
AndyAyersMS commentedon Feb 26, 2021
Not that I know of, no.
AndyAyersMS commentedon Feb 26, 2021
I don't expect us to get to this in .Net 6. So marking as future.
JIT: simple forward substitution pass (#63720)