-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Inline nullable allocators #122167
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
Inline nullable allocators #122167
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
Enabled structs too (basically, all types, even shared), so now #114497 (comment) is properly handed. |
|
@EgorBot -amd -arm using BenchmarkDotNet.Attributes;
[MemoryDiagnoser]
[DisassemblyDiagnoser]
public class Bench
{
private Nullable<bool> _null = null;
private Nullable<bool> _nonnull = true;
[Benchmark] public object BoxNull() => _null;
[Benchmark] public object BoxNonNull() => _nonnull;
// https://github.com/dotnet/runtime/issues/50915
private S? _ns = (S?)default(S);
[Benchmark] public int Issue50915() => CallM(_ns);
static int CallM<T>(T t)
{
if (t is IMyInterface)
return ((IMyInterface)t).M();
return 0;
}
}
interface IMyInterface
{
int M();
}
struct S : IMyInterface
{
public int M() => 42;
} |
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.
Pull request overview
This PR optimizes nullable boxing operations by introducing early inline expansion instead of using helper calls. The optimization enables escape analysis to stack-allocate boxed nullable values when they don't escape the method, eliminating heap allocations in hot paths. The implementation adds a new early QMARK expansion phase that runs after import but before other optimizations, allowing subsequent passes to optimize the expanded control flow.
Key changes:
- Inline expansion of nullable box operations in hot, optimized code paths using conditional allocation
- New early QMARK expansion phase to enable object stack allocation optimizations
- COMMA node splitting during QMARK expansion to improve optimization opportunities
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/importer.cpp | Adds inline expansion of nullable boxing with conditional allocation based on hasValue field |
| src/coreclr/jit/morph.cpp | Enhances QMARK expansion to support early phase, adds COMMA splitting, changes return type to PhaseStatus |
| src/coreclr/jit/compiler.cpp | Adds early QMARK expansion phase after import and updates late expansion call signature |
| src/coreclr/jit/compiler.h | Adds OMF_HAS_EARLY_QMARKS flag and updates fgExpandQmarkNodes signature |
| src/coreclr/jit/compphases.h | Defines new PHASE_EARLY_QMARK_EXPANSION phase |
c1431db to
ab99de1
Compare
23f505f to
69f6243
Compare
|
The superpmi diffs are obviously useless due to missing contexts. The normal jit-diff (e.g. this) is also a mess due to the way PMI works - we take all generics and try to instantiate them with one of the 7 predefined types (see here) and |
8e2a401 to
b2987b3
Compare
…nullable-allocators
|
It seems that the allocations are elided only for primitive types, for structs there's still an allocation: https://godbolt.org/z/aTxMocvhW |
This problem is not related to nullable, it's a general limitation of the escape analysis, can be reproduced via: static string Test1(Guid g)
{
object o = g;
return (o).ToString();
}
static string Test2(int g)
{
object o = g;
return (o).ToString();
}works for int, doesn't work for Guid 🙁 |
|
it will work if e.g. Guid.ToString is just a wrapper that calls a static |
|
But why is passing by reference a problem? It's still just a struct, so it can't escape anywhere... |
|
The example you provided doesn't actually allocate, it has something to do with |
Closes #50915
Today we never inline & stack-allocate (via the escape analysis) boxed Nullable<>, let's see if we can fix that.
Main:
PR:
Another example (from #114497 (comment)) cc @pentp
diff.
Doesn't allocate anymore to box that nullable.
Benchmarks: EgorBot/runtime-utils#563