dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.13k stars 4.7k forks source link

godbolt-specific improvements for crossgen #64981

Open EgorBo opened 2 years ago

EgorBo commented 2 years ago

Godbolt now supports C# via crossgen2 thanks to @hez2010, but it needs a couple of improvements for better UX πŸ™‚ Demo: https://godbolt.org/z/qaf3sPxTh

1) Faster "change C# -> observe codegen" loop. Currently it takes around 8s to observe codgen changes. This issue should be addressed via https://github.com/compiler-explorer/misc-builder/pull/27 and https://github.com/compiler-explorer/compiler-explorer/pull/3328 but if anyone has suggestions how to optimize it further I am all ears. Fun fact: DOTNET_TC_QuickJitForLoops=1 reduces the loop by 0.5s when applied to msbuild and crossgen (2s -> 1.5s on my machine). Looking forward to seeing OSR+QJFL enabled for .NET 7.0 πŸŽ‰

2) I suspect crossgen2 can do work faster if not asked to produce a real binary and only does --codegenopt:NgenDisasm=*

3) https://godbolt.org/z/adPfhTcK5 how can we hide this Nullability-related codegen?

4) DOTNET_JitDiffableDasm=1 hides too many details and constants, but DOTNET_JitDiffableDasm=1 prints raw instruction bytes we also rarely need. We need a config switch to hide raw instruction bytes only. Also, I don't think many people care about IL offsets.

5) line numbers https://github.com/compiler-explorer/compiler-explorer/pull/3334#issuecomment-1030932224

6) Enable JitDisasm and NgenDisasm in Release JIT - Checked JIT is too slow

category:testing theme:benchmarks skill-level:beginner cost:small impact:small

dotnet-issue-labeler[bot] commented 2 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

hez2010 commented 2 years ago

For line numbers support, I think we may use the IL offset in JIT dasm and map it back to C# code with the help of pdb files.

am11 commented 2 years ago

By using the same default as SharpLab? SharpLab has nullable set to disable by default. Interested users can enable it with pragma. Current workaround: https://godbolt.org/z/brzfW7qvT.

hez2010 commented 2 years ago

By using the same default as SharpLab? SharpLab has nullable set to disable by default. Interested users can enable it with pragma. Current workaround: https://godbolt.org/z/brzfW7qvT.

I think this doesn't matter, and nullable should be enabled by default. We can filter all unnecessary codegen result in asm-parser in the future. Check https://github.com/compiler-explorer/compiler-explorer/pull/3334 to see how asm-parser works.

EgorBo commented 2 years ago

By using the same default as SharpLab? SharpLab has nullable set to disable by default. Interested users can enable it with pragma. Current workaround: https://godbolt.org/z/brzfW7qvT.

I think this doesn't matter, and nullable should be enabled by default. We can filter all unnecessary codegen result in asm-parser in the future. Check compiler-explorer/compiler-explorer#3334 to see how asm-parser works.

I don't think we need nullability there, sharplab works much better for everything Roslyn related, disabled nullability means we prejit less

svick commented 2 years ago

Why is the nullability-related codegen missing method names? I think that indicates there is a problem there.

Another case where this happens is the simple class C { }, which shows just ret, when I think it should be C:.ctor():this: ret.

hez2010 commented 2 years ago

Why is the nullability-related codegen missing method names? I think that indicates there is a problem there.

Another case where this happens is the simple class C { }, which shows just ret, when I think it should be C:.ctor():this: ret.

That's an asm-parser bug and will be fixed soon.

ghost commented 2 years ago

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

Issue Details
Godbolt now supports C# via crossgen2 thanks to @hez2010, but it needs a couple of improvements for better UX πŸ™‚ Demo: https://godbolt.org/z/qaf3sPxTh 1) Faster "change C# -> observe codegen" loop. Currently it takes around 8s to observe codgen changes. This issue should be addressed via https://github.com/compiler-explorer/misc-builder/pull/27 and https://github.com/compiler-explorer/misc-builder/pull/27 but if anyone has suggestions how to optimize it further I am all ears. Fun fact: `DOTNET_TC_QuickJitForLoops=1` reduces the loop by 0.5s when applied to msbuild and crossgen (2s -> 1.5s on my machine). Looking forward to seeing OSR+QJFL enabled for .NET 7.0 πŸŽ‰ 2) I suspect crossgen2 can do work faster if not asked to produce a real binary and only does `--codegenopt:NgenDisasm=*` 3) https://godbolt.org/z/adPfhTcK5 how can we hide this Nullability-related codegen? 4) `DOTNET_JitDiffableDasm=1` hides too many details and constants, but `DOTNET_JitDiffableDasm=1` prints raw instruction bytes we also rarely need. We need a config switch to hide raw instruction bytes only. Also, I don't think many people care about IL offsets. 5) line numbers https://github.com/compiler-explorer/compiler-explorer/pull/3334#issuecomment-1030932224
Author: EgorBo
Assignees: -
Labels: `area-CodeGen-coreclr`, `untriaged`
Milestone: -
hez2010 commented 2 years ago

@EgorBo Which tier of code does crossgen2 actually generate? Seems that it sometimes but not always generates tier0 code, is there any way to configure this behavior to make it always produce tier1 code?

EgorBo commented 2 years ago

@EgorBo Which tier of code does crossgen2 actually generate? Seems that it sometimes but not always generates tier0 code, is there any way to configure this behavior to make it always produce tier1 code?

it should emit optimized code unless --optimize-disabled is provided, can you share some snippet where you encountered a bad codegen?

hez2010 commented 2 years ago

it should emit optimized code unless --optimize-disabled is provided, can you share some snippet where you encountered a bad codegen?

Never mind πŸ˜…. It turns out to be a difference between Windows and Linux codegen with same quality. See https://godbolt.org/z/To6Pone8v. Is this expected?

JulieLeeMSFT commented 2 years ago

Look at only #1 in .NET 7. The rest of work items can be done in future.

ShreyasJejurkar commented 1 year ago

Hello all, just wanted to know with .NET 7 we added ways via which users can see underlying generated machine code. Can we use that capability to show off machine code in compiler explorer today!? Because I just gave try to generate machine code for simple code, but it still takes a lot of time for compilation, and then it shows machine code. Not sure right now under the hood we use those options in .NET for compiler explorer, but I guess that enabling that would reduce compilation time in CE.

ShreyasJejurkar commented 1 year ago

I see this PR got closed a long time ago, are there any chances we can revive this and get merged!? https://github.com/compiler-explorer/compiler-explorer/pull/3328

cc @EgorBo

hez2010 commented 1 year ago

I see this PR got closed a long time ago, are there any chances we can revive this and get merged!? https://github.com/compiler-explorer/compiler-explorer/pull/3328

cc @EgorBo

Pregenerated projects with pre-restoring doesn't work in the compiler explorer environment. Other improvements have been merged in another PR.

ShreyasJejurkar commented 1 year ago

So there are no ways as of now to improve the turnaround time at compiler explorer!?

hez2010 commented 1 year ago

There should still be some rooms for improvement. For now most time is spending on crossgen2, maybe we can switch to corerun with a custom type loader in the future.

hez2010 commented 1 week ago

Most work have been done now, modulus line number support.

With additional items done:

am11 commented 1 week ago

Thanks @hez2010, the NativeAOT asm output looks exceptionally clean in the latest release! πŸ‘

It seems the --targetarch arm64 and --targetos type of compiler options are not functioning as before. Perhaps I am forgetting the previous behavior and they might still be operational?

hez2010 commented 1 week ago

It seems the --targetarch arm64 and --targetos type of compiler options are not functioning as before. Perhaps I am forgetting the previous behavior and they might still be operational?

You can only use them with crossgen2. For NativeAOT we only have linux-x64 runtime pack and rootfs so it doesn't support cross-compilation.