JIT does not inline operators against generic math

[hez2010]

Description

I'm implementing a generic Point<T> struct with below code:

record struct Point<T>(T X, T Y) : INumberBase<Point<T>> where T : INumberBase<T>
{
    public static Point<T> One => new(T.One, T.One);
    public static Point<T> Zero => new(T.Zero, T.Zero);
    public static Point<T> AdditiveIdentity => new(T.Zero, T.Zero);
    public static Point<T> MultiplicativeIdentity => new(T.One, T.One);
    public static Point<T> operator checked +(Point<T> left, Point<T> right) => new (checked(left.X + right.X), checked(left.Y + right.Y));
    public static Point<T> operator checked -(Point<T> value) => new(checked(-value.X), checked(-value.Y));
    public static Point<T> operator checked -(Point<T> left, Point<T> right) => new(checked(left.X - right.X), checked(left.Y - right.Y));
    public static Point<T> operator checked ++(Point<T> value) => checked(value + One);
    public static Point<T> operator checked --(Point<T> value) => checked(value - One);
    public static Point<T> operator checked *(Point<T> left, Point<T> right) => new(checked(left.X * right.X), checked(left.Y * right.Y));
    public static Point<T> operator checked /(Point<T> left, Point<T> right) => new(checked(left.X / right.X), checked(left.Y / right.Y));
    public string ToString(string? format, IFormatProvider? formatProvider) => $"({X.ToString(format, formatProvider)}, {Y.ToString(format, formatProvider)})";
    public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider) => throw new NotImplementedException();
    public static Point<T> operator +(Point<T> value) => value;
    public static Point<T> operator +(Point<T> left, Point<T> right) => new(left.X + right.X, left.Y + right.Y);
    public static Point<T> operator -(Point<T> value) => new(-value.X, -value.Y);
    public static Point<T> operator -(Point<T> left, Point<T> right) => new(left.X - right.X, left.Y - right.Y);
    public static Point<T> operator ++(Point<T> value) => value + One;
    public static Point<T> operator --(Point<T> value) => value - One;
    public static Point<T> operator *(Point<T> left, Point<T> right) => new(left.X * right.X, left.Y * right.Y);
    public static Point<T> operator /(Point<T> left, Point<T> right) => new(left.X / right.X, left.Y / right.Y);
}

But when I test it with a simple snippet:

var p1 = new Point<int>(3, 4);
var p2 = new Point<int>(5, 6);
return (p1 + p2).X + (p1 + p2).Y;

The JIT fails to inline operator+ thus it produces extremely bad codegen:

push      rsi
sub       rsp,40
mov       dword ptr [rsp+30],3
mov       dword ptr [rsp+34],4
mov       dword ptr [rsp+28],5
mov       dword ptr [rsp+2C],6
mov       rcx,[rsp+30]
mov       rdx,[rsp+28]
call      qword ptr [7FFF8E0B7CA8]
mov       [rsp+38],rax
mov       esi,[rsp+38]
mov       dword ptr [rsp+30],3
mov       dword ptr [rsp+34],4
mov       dword ptr [rsp+28],5
mov       dword ptr [rsp+2C],6
mov       rcx,[rsp+30]
mov       rdx,[rsp+28]
call      qword ptr [7FFF8E0B7CA8]
mov       [rsp+38],rax
mov       eax,esi
add       eax,[rsp+3C]
add       rsp,40
pop       rsi
ret

Only if I put [MethodImpl(MethodImplOptions.AggressiveInlining)] will the codegen be optimal:

mov       eax,12
ret

And the performance difference:

Method	Mean	Error	StdDev	Code Size
Test_NoInlining	23.12 ns	0.499 ns	0.820 ns	172 B
Test_Inlining	0.0099 ns	0.0145 ns	0.0332 ns	6 B

It's 2335x slower compared to the inlined one! Note that turning on PGO does not help in this case.
Operators are usually extremely hot path so I expect them to be inlined.

IL for operator+:

IL_0000:	ldarga.s	00 
IL_0002:	call	Point <T>.get_X ()
IL_0007:	ldarga.s	01 
IL_0009:	call	Point <T>.get_X ()
IL_000E:	constrained.	Point <T>.T
IL_0014:	call	IAdditionOperators <T, T, T>.op_Addition (T, T)
IL_0019:	ldarga.s	00 
IL_001B:	call	Point <T>.get_Y ()
IL_0020:	ldarga.s	01 
IL_0022:	call	Point <T>.get_Y ()
IL_0027:	constrained.	Point <T>.T
IL_002D:	call	IAdditionOperators <T, T, T>.op_Addition (T, T)
IL_0032:	newobj	Point <T>..ctor
IL_0037:	ret	

Configuration

.NET 7: build from master

category:cq
theme:inlining
skill-level:intermediate
cost:medium
impact:medium

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

Issue Details

---

Description

I'm implementing a generic Point<T> struct with below code:

record struct Point<T>(T X, T Y) : INumberBase<Point<T>> where T : INumberBase<T>
{
    public static Point<T> One => new(T.One, T.One);
    public static Point<T> Zero => new(T.Zero, T.Zero);
    public static Point<T> AdditiveIdentity => new(T.One, T.One);
    public static Point<T> MultiplicativeIdentity => new(T.One, T.One);
    public static Point<T> operator checked +(Point<T> left, Point<T> right) => new (checked(left.X + right.X), checked(left.Y + right.Y));
    public static Point<T> operator checked -(Point<T> value) => new(checked(-value.X), checked(-value.Y));
    public static Point<T> operator checked -(Point<T> left, Point<T> right) => new(checked(left.X - right.X), checked(left.Y - right.Y));
    public static Point<T> operator checked ++(Point<T> value) => checked(value + One);
    public static Point<T> operator checked --(Point<T> value) => checked(value - One);
    public static Point<T> operator checked *(Point<T> left, Point<T> right) => new(checked(left.X * right.X), checked(left.Y * right.Y));
    public static Point<T> operator checked /(Point<T> left, Point<T> right) => new(checked(left.X / right.X), checked(left.Y / right.Y));
    public string ToString(string? format, IFormatProvider? formatProvider) => $"({X.ToString(format, formatProvider)}, {Y.ToString(format, formatProvider)})";
    public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider) => throw new NotImplementedException();
    public static Point<T> operator +(Point<T> value) => value;
    public static Point<T> operator +(Point<T> left, Point<T> right) => new(left.X + right.X, left.Y + right.Y);
    public static Point<T> operator -(Point<T> value) => new(-value.X, -value.Y);
    public static Point<T> operator -(Point<T> left, Point<T> right) => new(left.X - right.X, left.Y - right.Y);
    public static Point<T> operator ++(Point<T> value) => value + One;
    public static Point<T> operator --(Point<T> value) => value - One;
    public static Point<T> operator *(Point<T> left, Point<T> right) => new(left.X * right.X, left.Y * right.Y);
    public static Point<T> operator /(Point<T> left, Point<T> right) => new(left.X / right.X, left.Y / right.Y);
}

But when I test it with a simple snippet:

var p1 = new Point<int>(3, 4);
var p2 = new Point<int>(5, 6);
return (p1 + p2).X + (p1 + p2).Y;

The JIT failed to inline operator+ thus it produces extremely bad codegen:

push      rsi
sub       rsp,40
mov       dword ptr [rsp+30],3
mov       dword ptr [rsp+34],4
mov       dword ptr [rsp+28],5
mov       dword ptr [rsp+2C],6
mov       rcx,[rsp+30]
mov       rdx,[rsp+28]
call      qword ptr [7FFF8E0B7CA8]
mov       [rsp+38],rax
mov       esi,[rsp+38]
mov       dword ptr [rsp+30],3
mov       dword ptr [rsp+34],4
mov       dword ptr [rsp+28],5
mov       dword ptr [rsp+2C],6
mov       rcx,[rsp+30]
mov       rdx,[rsp+28]
call      qword ptr [7FFF8E0B7CA8]
mov       [rsp+38],rax
mov       eax,esi
add       eax,[rsp+3C]
add       rsp,40
pop       rsi
ret

Only if I put [MethodImpl(MethodImplOptions.AggressiveInlining)] will the codegen be optimal:

mov       eax,12
ret

And the performance difference:

Method	Mean	Error	StdDev	Median	Code Size
Test_NoInlining	23.12 ns	0.499 ns	0.820 ns	172 B
Test_Inlining	0.0099 ns	0.0145 ns	0.0332 ns	0.0 ns	6 B

It's 2335x slower compared to the inlined one!
Operators are usually extreme hot path so I expect them to be inlined.

Configuration

.NET 7: build from master

Author:	hez2010
Assignees:	-
Labels:	tenet-performance, area-CodeGen-coreclr
Milestone:	-

[hez2010]

weight= 77 : state  16 [ ldarga.s ]
weight= 79 : state  40 [ call ]
weight= 77 : state  16 [ ldarga.s ]
weight= 79 : state  40 [ call ]
weight= 65 : state 181 [ constrained ]
weight= 79 : state  40 [ call ]
weight= 77 : state  16 [ ldarga.s ]
weight= 79 : state  40 [ call ]
weight= 77 : state  16 [ ldarga.s ]
weight= 79 : state  40 [ call ]
weight= 65 : state 181 [ constrained ]
weight= 79 : state  40 [ call ]
weight=227 : state 103 [ newobj ]
weight= 19 : state  42 [ ret ]

multiplier in methods of struct increased to 3.
Inline candidate returns a struct by value.  Multiplier increased to 5.
Inline candidate is generic and caller is not.  Multiplier increased to 7.
Inline candidate callsite is boring.  Multiplier increased to 8.3.
calleeNativeSizeEstimate=1158
callsiteNativeSizeEstimate=115
benefit multiplier=8.3
threshold=954
Native estimate for function size exceeds threshold for inlining 115.8 > 95.4 (multiplier = 8.3)

[tannergooding]

cc. @AndyAyersMS, @EgorBo

Part of this is due to using properties rather than direct field accesses (a side effect of it being an auto-prop from the record).

Them being instance methods doesn't help either since it requires address taken each time.

We should probably handle this type of scenario better, particular since it's impactful to common higher level patterns and will be more prominent for generic math (one of the bigger .NET 7 features).

Making an assumption that properties are cheap and generally inlineable might be a good start. Doing similar for operators might also be reasonable. I don't imagine we could make any assumptions about other call types without some kind of analysis or tracked data to improve the heuristics.

[hez2010]

The current inliner is really bad at handling virtual statics, even a simple snippet could result in inline failure:

using System;
using System.Runtime.CompilerServices;

Foo<MyFoo>();

// not auto-inlined
static void Foo<T>() where T : IFoo, IBar
{
    Console.WriteLine(T.Foo() + T.Bar());
}

interface IFoo
{
    abstract static int Foo();
}

interface IBar
{
    abstract static int Bar();
}

class MyFoo : IFoo, IBar
{
    public static int Foo() => 3;
    public static int Bar() => 4;
}

[tannergooding]

The current inliner is really bad at handling virtual statics, even a simple snippet could result in inline failure:

This isn't really anything to do with virtual statics. This has to do with there being 3 method calls and 29 bytes of IL which makes it look costly:

        IL_0000: constrained. !!T
        IL_0006: call int32 IFoo::Foo()
        IL_000b: constrained. !!T
        IL_0011: call int32 IBar::Bar()
        IL_0016: add
        IL_0017: call void [System.Console]System.Console::WriteLine(int32)
        IL_001c: ret

You'll note that this isn't inlined either:

using System;
using System.Runtime.CompilerServices;

Foo();

// not auto-inlined
static void Foo()
{
    Console.WriteLine(MyFoo.Foo() + MyFoo.Bar());
}

class MyFoo
{
    public static int Foo() => 3;
    public static int Bar() => 4;
}

The issue is that the JIT only sees a call instruction + a method token. It is decently expensive to resolve that method token and more expensive to also resolve the IL, get a size estimate, and do heuristical analysis on that as well.

So the JIT effectively just sees "Main has 1 call, that call is n bytes of IL and contains 3 calls itself, none of which are intrinsic or otherwise look interesting". This causes it to believe the call is not interesting to inline and that it is more expensive than it actually is.

[AndyAyersMS]

Making an assumption that properties are cheap and generally inlineable might be a good start. Doing similar for operators might also be reasonable.

I suspect we'd find that this leads to quite a bit of unprofitable code bloat. I think we need a way to identify cases like these that are more likely amenable to optimization—for instance if we knew that these properties/methods were connected to generic math. Currently the jit has no idea what interfaces a type may implement.

For general heuristics you could certainly argue that at least the constrained. prefix and associated token shouldn't really count against the IL size.

[tannergooding]

I suspect we'd find that this leads to quite a bit of unprofitable code bloat

I'd be surprised if this lead to unprofitable code bloat, at least for properties.

I expect we'd find the vast majority of properties are auto-props or simple field accesses, sometimes with simple initialization logic in the worst case. These represent what is functionally a simple method call over a field read/write and so its likely safe to make an initial assumption that they are less expensive than an actual call (I'd expect most are within the "always inline" limit even).

Operators are a bit tougher because you can range from simple to complex and everything in between. I expect we'd find that giving them a small multiplier (and similar for any other [SpecialName] methods) would be generally more profitable then not and those that are actually too big will end up falling out otherwise.

[AndyAyersMS]

I'd expect most are within the "always inline" limit even

Right—trivial properties are already going to be inlined, so what's left discretionary are the nontrivial ones. Last I looked (which was admittedly a while ago) there was a pretty steep size curve increasing the nondiscretionary set in any way. We likely need to look for other evidence of profitability.

The jit doesn't know about special name methods. We could flag those on the runtime side if needed (via getMethodAttribs say).

[tannergooding]

Right—trivial properties are already going to be inlined, so what's left discretionary are the nontrivial ones.

Right, when considering whether get_Foo() should be inlined into its caller Foo(). However, when considering whether to inline Foo() into it's caller, this is where trivial properties are currently problematic.

If we take a look at:

using System;

Foo();

// not auto-inlined
static void Foo()
{
    Console.WriteLine(MyFoo.Foo + MyFoo.Bar);
}

class MyFoo
{
    public static int Foo => 3;
    public static int Bar => 4;
}

We'll look at Foo() and it's IL will be:

        IL_0000: call int32 MyFoo::get_Foo()
        IL_0005: call int32 MyFoo::get_Bar()
        IL_000a: add
        IL_000b: call void [System.Console]System.Console::WriteLine(int32)
        IL_0010: ret

We note that its 17 bytes of IL with 3 calls. We now even resolve the method tokens for these to check if they are intrinsic or anything else interesting. However, since we don't currently do anything special for properties we assume these calls are decently expensive and that negatively impacts things here.

My proposal is that we have the inliner give a small profitability multiplier when it encounters property getters/setters under the presumption that most properties are "trivial" and are roughly simple field accesses. Getting that information from the methodHnd is relatively straightforward and I'd expect it would help such scenarios.

[JulieLeeMSFT]

cc @dotnet/jit-contrib.