Skip to content
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

Use RyuJIT as IL scanner in NativeAOT #83021

Open
MichalStrehovsky opened this issue Mar 6, 2023 · 2 comments
Open

Use RyuJIT as IL scanner in NativeAOT #83021

MichalStrehovsky opened this issue Mar 6, 2023 · 2 comments

Comments

@MichalStrehovsky
Copy link
Member

Definitely Future milestone - just wanted to write down a couple notes as I spent a couple hours looking at this today.

Optimized compilation in NativeAOT consists of two phases - scanning (builds whole program view) and compilation (generates code).

Scanning is currently implemented in C# - we have an IL importer that essentially simulates some things that RyuJIT will need when compiling the method (e.g. method call will require us to provide method body of the callee, etc.).

We discussed a couple times doing this analysis with RyuJIT and there even was a prototype that didn't quite do what we need in NativeAOT (it collected whole program view for RyuJIT's purposes, but not ours, so it's not very useful for NativeAOT - what we need is the list of relocs from a method body).

I put together a hack to let us run RyuJIT as a scanner: MichalStrehovsky@34f7a96

This hack does nothing to prevent codegen, so we do all the unnecessary things like register allocation and code generation in RyuJIT and then we throw it all away. The compile throughput impact is about 15%.

It is a regression in size, both for BasicMinimalApi (10%) and HelloWorld (5%). I expect it also to regress working set as a side effect.

Observations:

  • We don't have a good way to model optimizations we currently do in scanner through JitInterface (e.g. someType.GetType() == typeof(X) currently avoids requesting a MethodTable with a populated vtable in the C# scanner, but RyuJIT can't communicate a limited MethodTable is sufficient - we get a full one).
  • Fix VNs for method table fetched from newobj runtimelab#1128 shows up as a problem again because we want to be able to give out MethodTable symbols without a vtable
  • RyuJIT may optimize away some things during importing around virtual method lookups, or generic dictionary lookup, but then during compilation it's going to ask questions about those that we cannot answer (this structurally falls out from JitInterface design where getCallinfo provides some information that RyuJIT may optimize into something that doesn't need the thing JitInterface provided).

Not sure if all of the size regression can be attributed to above - the above causes enough noise that it wasn't worth spending more time on it.

@ghost
Copy link

ghost commented Mar 6, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Definitely Future milestone - just wanted to write down a couple notes as I spent a couple hours looking at this today.

Optimized compilation in NativeAOT consists of two phases - scanning (builds whole program view) and compilation (generates code).

Scanning is currently implemented in C# - we have an IL importer that essentially simulates some things that RyuJIT will need when compiling the method (e.g. method call will require us to provide method body of the callee, etc.).

We discussed a couple times doing this analysis with RyuJIT and there even was a prototype that didn't quite do what we need in NativeAOT (it collected whole program view for RyuJIT's purposes, but not ours, so it's not very useful for NativeAOT - what we need is the list of relocs from a method body).

I put together a hack to let us run RyuJIT as a scanner: MichalStrehovsky@34f7a96

This hack does nothing to prevent codegen, so we do all the unnecessary things like register allocation and code generation in RyuJIT and then we throw it all away. The compile throughput impact is about 15%.

It is a regression in size, both for BasicMinimalApi (10%) and HelloWorld (5%). I expect it also to regress working set as a side effect.

Observations:

  • We don't have a good way to model optimizations we currently do in scanner through JitInterface (e.g. someType.GetType() == typeof(X) currently avoids requesting a MethodTable with a populated vtable in the C# scanner, but RyuJIT can't communicate a limited MethodTable is sufficient - we get a full one).
  • Fix VNs for method table fetched from newobj runtimelab#1128 shows up as a problem again because we want to be able to give out MethodTable symbols without a vtable
  • RyuJIT may optimize away some things during importing around virtual method lookups, or generic dictionary lookup, but then during compilation it's going to ask questions about those that we cannot answer (this structurally falls out from JitInterface design where getCallinfo provides some information that RyuJIT may optimize into something that doesn't need the thing JitInterface provided).

Not sure if all of the size regression can be attributed to above - the above causes enough noise that it wasn't worth spending more time on it.

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: 8.0.0

@MichalStrehovsky
Copy link
Member Author

It might be beneficial to be able to scan methods basic block after basic block to fix issues like #92850.

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jan 17, 2024
@stephentoub found out that for following code:

```csharp
using System.Buffers;

Foo<Bar>();

static T[] Foo<T>()
{
    if (typeof(T).IsValueType)
    {
        return ArrayPool<T>.Shared.Rent(42);
    }
    return null!;
}

class Bar {}
```

We end up generating `ArrayPool`s of `Bar` even though it's obviously never reachable. The problem is architectural:

* We run a whole program analysis phase that tries to figure out things like generic dictionary layouts so that later, in code generation phase, we can inline offsets into generic dictionaries into codegen.
* For the above code, whole program analysis decides that the dictionary layout of `Foo<__Canon>` needs a slot for `ArrayPool<!0>`.
* Codegen then optimizes out the `IsValueType` branch because `__Canon` is never a valuetype. But we already allocated the dictionary slot and will fill it out, even though it ends up unused due to the optimization.

We're going to run into issues like this until dotnet#83021 is addressed. Whole program analysis cannot currently assume a certain optimization happens because we don't know whether RyuJIT will do it. The only way we can "optimize" during whole program analysis is if we rewrite IL and give RyuJIT no saying in whether to do an optimization or not. Rewriting the IL is not great because it e.g. causes PGO data to not match. I don't like doing it, but there's nothing else we can do.

This change extends dead block elimination to understand `typeof(X).IsValueType`. If we recognize a branch is reachable under this condition, we evaluate whether this is true or false and replace the basic block with nops.
MichalStrehovsky added a commit that referenced this issue Jan 18, 2024
@stephentoub found out that for following code:

```csharp
using System.Buffers;

Foo<Bar>();

static T[] Foo<T>()
{
    if (typeof(T).IsValueType)
    {
        return ArrayPool<T>.Shared.Rent(42);
    }
    return null!;
}

class Bar {}
```

We end up generating `ArrayPool`s of `Bar` even though it's obviously never reachable. The problem is architectural:

* We run a whole program analysis phase that tries to figure out things like generic dictionary layouts so that later, in code generation phase, we can inline offsets into generic dictionaries into codegen.
* For the above code, whole program analysis decides that the dictionary layout of `Foo<__Canon>` needs a slot for `ArrayPool<!0>`.
* Codegen then optimizes out the `IsValueType` branch because `__Canon` is never a valuetype. But we already allocated the dictionary slot and will fill it out, even though it ends up unused due to the optimization.

We're going to run into issues like this until #83021 is addressed. Whole program analysis cannot currently assume a certain optimization happens because we don't know whether RyuJIT will do it. The only way we can "optimize" during whole program analysis is if we rewrite IL and give RyuJIT no saying in whether to do an optimization or not. Rewriting the IL is not great because it e.g. causes PGO data to not match. I don't like doing it, but there's nothing else we can do.

This change extends dead block elimination to understand `typeof(X).IsValueType`. If we recognize a branch is reachable under this condition, we evaluate whether this is true or false and replace the basic block with nops.
tmds pushed a commit to tmds/runtime that referenced this issue Jan 23, 2024
@stephentoub found out that for following code:

```csharp
using System.Buffers;

Foo<Bar>();

static T[] Foo<T>()
{
    if (typeof(T).IsValueType)
    {
        return ArrayPool<T>.Shared.Rent(42);
    }
    return null!;
}

class Bar {}
```

We end up generating `ArrayPool`s of `Bar` even though it's obviously never reachable. The problem is architectural:

* We run a whole program analysis phase that tries to figure out things like generic dictionary layouts so that later, in code generation phase, we can inline offsets into generic dictionaries into codegen.
* For the above code, whole program analysis decides that the dictionary layout of `Foo<__Canon>` needs a slot for `ArrayPool<!0>`.
* Codegen then optimizes out the `IsValueType` branch because `__Canon` is never a valuetype. But we already allocated the dictionary slot and will fill it out, even though it ends up unused due to the optimization.

We're going to run into issues like this until dotnet#83021 is addressed. Whole program analysis cannot currently assume a certain optimization happens because we don't know whether RyuJIT will do it. The only way we can "optimize" during whole program analysis is if we rewrite IL and give RyuJIT no saying in whether to do an optimization or not. Rewriting the IL is not great because it e.g. causes PGO data to not match. I don't like doing it, but there's nothing else we can do.

This change extends dead block elimination to understand `typeof(X).IsValueType`. If we recognize a branch is reachable under this condition, we evaluate whether this is true or false and replace the basic block with nops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

1 participant