NativeAOT-LLVM: RyuJIT IR -> LLVM for improved performance #647

yowl · 2021-02-06T20:25:50Z

Some mucking around to try to get something that can be played with. Copied a lot of clrjit macros and code from xarch/amd64 and deleted/changed to get compilation to succeed and run at least as far as compiling the first method. This is my attempt to tackle #313 and may be total garbage. The clrjit does nothing at the moment exception assert(false) at the appropriate phase. In theory it should be possible to catch this and revert to CIL -> LLVM allowing tests to run at an early stage of playing while adding IR support.

…orRunner} > clrjit_unix_x64_x64.dll!Compiler::getPrimitiveTypeForStruct(unsigned int structSize, CORINFO_CLASS_STRUCT_ * clsHnd, bool isVarArg) Line 723 C++ with Run-Time Check Failure dotnet#2 - Stack around the variable 'gcPtr' was corrupted.

Wasm64 has some macros defined, but otherwise not compiled Copied mostly from the xarch and AMD64 macros to get something to compile and run.

jkotas · 2021-02-07T04:11:08Z

cc @EgorBo

jkotas · 2021-02-07T04:13:20Z

src/coreclr/inc/corinfoinstructionset.h

+    //InstructionSet_SSE3 = 4,
+    //InstructionSet_SSSE3 = 5,
+    //InstructionSet_SSE41 = 6,
+    InstructionSet_SSE42 = 7,


It does not sound right that you have to define SSE2 and AVX2 for wasm

They were getting used for the SIMD check

runtimelab/src/coreclr/jit/compiler.h

Line 8038 in fdc65fc

SIMDLevel getSIMDSupportLevel()

. Wasm can support SIMD, for c++ emscripten handles things. for LLVM I've not looked what might be necessary, but it would be good if the clrjit optimisation for SIMD that occurs before lowering, if any, was still in I suppose. For now this method is an assert for Wasm and these instructions in corinfoinstructionset.h are removed.

I would expect Wasm to have its own wasm-specific names for SIMD levels, and not to reuse the x86/x64 ones. ARM does not reuse the x86/x64 ones either.

I agree. https://github.com/WebAssembly/simd isn't finalized yet, but when it is I'd expect we would look at introducing a new System.Runtime.Intrinsics.Wasm namespace and corresponding ISAs.

Ok, for now I've just deleted this change. https://github.com/dotnet/runtimelab/pull/647/files#diff-5bc0c93aea7b72e9e37d8742e7a59ffb882ff206b746494f2e1f484bc3ef47c1

jkotas · 2021-02-07T04:17:59Z

src/coreclr/jit/emit.h

@@ -576,6 +576,10 @@ class emitter
 #elif defined(TARGET_ARM64)


CodeGen and emitter are late phases. They should be all replaced by the LLVM part. Is it possible to ifdef them out?

Do you think I should be able #if out the Lowering class? https://github.com/dotnet/runtimelab/blob/feature/NativeAOT/src/coreclr/jit/lower.h
Edit... I will go for removing it as well as the lsra code as that seems to make sense as at least the SIMPLE_LOWERING is a late phase and assume Lowering is part of that, or something later.

Have removed those + lsra + lower. There's a few places that require #ifdefs to allow compilation to succeed. In some places where I'm not sure what's best I've left the wasm build throwing an assert.

I think inst* can be remove too.

Thanks, removed.

wasm compilation will now complete, although has an issue with the dbg instruction in Array Resize

Some code alerts false where not sure what to do. Passes non-wasm tests locally.

jkotas · 2021-02-08T07:22:46Z

src/coreclr/jit/lsra.h

@@ -735,7 +735,7 @@ class LinearScan : public LinearScanInterface
    // Hence the "SmallFPSet" has 5 elements.
    CLANG_FORMAT_COMMENT_ANCHOR;

-#if defined(TARGET_AMD64)
+#if defined(TARGET_AMD64) || defined(TARGET_WASM)


There changes should not be needed now that the whole file is ifdefed out

jkotas · 2021-02-08T07:27:51Z

src/coreclr/jit/stacklevelsetter.cpp

    // The constructor reads this value to skip iterations that could set it if it is already set.
    compiler->codeGen->resetWritePhaseForFramePointerRequired();
+#else
+    assert(false); // Wasm - TODO can this be ignored?


It should be possible to exclude the whole StackLevelSetter (it runs after lowering).

Thanks, yes, removed also.

remove stacklevelsetter with ifdef fix ifdef syntax

yowl · 2021-02-10T00:05:00Z

@jkotas For the wasm build, I need to build the clrjit for the host platform, initially win64. The gen-buildsys script at the moment uses emcmake to configure the CMake system for emscripten, but I don't want that for clrjit. One thought I have is to just run the build twice, run build-runtime.cmd twice I suppose, once for wasm and once for the host, possibly disabling as much as possible on the host build so that just the jitinterface and clrjit is build. The other option which might be possible, it to use __BuildCrossArchNative but I'm not sure if this is really the intended use for that?

jkotas · 2021-02-10T05:09:54Z

I am not sure which one of these options will turn out better. I think either one can work fine.

…oment attempt to add a second platform to the wasm yaml build fix problem where Array<T>.Resize has its code compiled twice.

yowl · 2021-02-21T19:27:58Z

At

runtimelab/src/coreclr/inc/crosscomp.h

Line 385 in fab1157

#define T_CONTEXT CONTEXT

It defines T_CONTEXT. For windows this comes from winnt.h in the SDK, but for Wasm what should I do here? It looks like its mostly registers, so would an empty

typedef struct DECLSPEC_ALIGN(16) DECLSPEC_NOINITALL _CONTEXT {
} CONTEXT, *PCONTEXT;

Be somewhere to start?

jkotas · 2021-02-21T19:32:51Z

Yes, a dummy definitions like this sound like a good way to start. You can omit DECLSPEC_ALIGN(16).

This whole header should not be really included by the JIT (there are only a very few things - if any - needed by the JIT from this header), but refactoring to fix that may be too involved.

May also need before including any LLVM header files: #undef NumItems

…jit-llvm-imethodcodenode # Conflicts: # src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp # src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp # src/coreclr/inc/icorjitinfoimpl_generated.h # src/coreclr/inc/jiteeversionguid.h # src/coreclr/jit/ICorJitInfo_API_wrapper.hpp # src/coreclr/jit/instr.cpp # src/coreclr/jit/target.h # src/coreclr/tools/Common/JitInterface/CorInfoBase.cs # src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt # src/coreclr/tools/aot/ILCompiler.LLVM/Compiler/LLVMCodegenCompilation.cs # src/coreclr/tools/aot/ILCompiler.LLVM/Compiler/LLVMCodegenCompilationBuilder.cs # src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs # src/coreclr/tools/aot/jitinterface/jitinterface.h Workaround for min, max, and NumItems #defines.

Fix up some merge problems

additional exports for llvm clrjit

# Conflicts: # src/coreclr/jit/CMakeLists.txt

…that is the compression Llvm use) Add build of clrjit for Windows x64 Debug only. This means that WebAssembly can only be built on that combination (as is the tested case currently).

enable linking from publish some refactor to allow the signature to be retrieved to create the externals for clrjit methods.

…jit-llvm-imethodcodenode # Conflicts: # src/coreclr/tools/aot/ILCompiler.LLVM/CodeGen/ILToLLVMImporter.cs

yowl · 2021-03-16T17:27:37Z

@jkotas This has gotten pretty long in number of commits, especially the attempts to get the CI working, which I tried to squash, but must have failed. Do you want me to squash the whole lot?

Its ready for review now that the nuget package is working and the hard coded method is removed (it does 9 methods currently, which is all the nop;ret void that it encounters). I put in a bit of a temporary fudge in src\coreclr\nativeaot\BuildIntegration\Microsoft.NETCore.Native.targets to handle the fact that while coverage is brought to 100% there will be 2 native object files.

From here I was going to suggest that coverage of LIR is done in smaller PRs, each one adding to the % of method coverage.

jkotas

Thank you for pushing this forward!

docs/workflow/building/coreclr/nativeaot.md

jkotas · 2021-03-16T19:08:36Z

src/coreclr/jit/codegencommon.cpp

@@ -30,6 +30,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 #include "patchpointinfo.h"

 /*****************************************************************************/
+#endif //!TARGET_WASM

 const BYTE genTypeSizes[] = {


I may be nice to move this to compiler.cpp since they are declared in compiler.h/hpp

(This change can be pushed to dotnet/runtime as general goodness.)

Thanks,
If I've got this right at https://github.com/dotnet/runtimelab/pull/647/commits
/76fb9b5a3ffc78b4164526e4a4f706b4fac3cbe9 then I'll do the runtime PR

jkotas · 2021-03-16T20:10:16Z

src/coreclr/inc/switches.h

@@ -65,6 +65,8 @@
    #define USE_UPPER_ADDRESS       0
 #endif // !HOST_UNIX

+#elif defined(TARGET_WASM)
+    #define USE_UPPER_ADDRESS       0 // TODO : what's this?


This controls how VM allocates memory for executable code. JIT should not really depend on it.

thanks, have left it in, but updated the comment

jkotas · 2021-03-16T21:08:49Z

src/coreclr/jit/compiler.cpp

+#if defined(TARGET_WASM)
+#include "llvm.h"
+#else
+// TODO: how to get different exports.def for the different clrjits?


This is a workaround. It should be a method on the JIT/EE interface eventually that will make this problem go away.

I handled the exports.def in the CMakeLists.txt so deleted this comment.

jkotas · 2021-03-16T21:10:01Z

src/coreclr/jit/compiler.cpp

+    Llvm* llvm = new Llvm();
+    llvm->Compile(pCompiler);
+    delete llvm;
+    //assert(false);


Suggested change

//assert(false);

jkotas · 2021-03-17T03:41:18Z

src/coreclr/jit/typelist.h

@@ -3,7 +3,11 @@

 #define GCS EA_GCREF
 #define BRS EA_BYREF
+#if defined(TARGET_WASM)
+#define PS TARGET_POINTER_SIZE


This can be TARGET_POINTER_SIZE unconditionally

ok, push to runtime?

Yes, that may be nice.

jkotas · 2021-03-17T03:42:17Z

src/coreclr/jit/unwindwasm.cpp

+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+/*XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX


This should be only called from codegen. None of it should be needed (or be reachable).

jkotas · 2021-03-17T03:43:42Z

src/coreclr/tools/aot/ILCompiler.LLVM/CodeGen/ILToLLVMImporter.cs

@@ -173,6 +175,74 @@ public ILImporter(LLVMCodegenCompilation compilation, MethodDesc method, MethodI
            _builder = Context.CreateBuilder();
        }

+//         [DllImport(JitSupportLibrary)]


yes, done, thanks

jkotas · 2021-03-17T03:44:33Z

src/coreclr/tools/aot/ILCompiler.LLVM/Compiler/RyuJitLLVMCompilation.cs

@@ -0,0 +1,200 @@
+// // Licensed to the .NET Foundation under one or more agreements.
+// // The .NET Foundation licenses this file to you under the MIT license.
+//


Delete? The whole file is commented out.

jkotas · 2021-03-17T03:52:10Z

src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.Llvm.cs

+        [UnmanagedCallersOnly]
+        public static byte* getMangledMethodName(IntPtr thisHandle, CORINFO_METHOD_STRUCT_* ftn)
+        {
+            //var _this = GetThis(thisHandle); // TODO: this doesn't work, but how does it cope anyway with this being moved by the GC?


but how does it cope anyway with this being moved by the GC?

The IntPtr points to stack location where CorInfoImpl reference is stored.

JIT/EE interface assumes that the JIT does not store the this pointer in a static. registerLlvmCallbacks stores the this pointer in a static. I suspect that it is why it does not work.

Thanks for the explanation, have done something similar so this now works without the static.

Co-authored-by: Jan Kotas <[email protected]>

…/runtimelab into ryujit-llvm-imethodcodenode

- Remove unnecessary ifdefs - Replace ifdefs with cmake exclusions - Fix passing of corinfo - remove some comments

yowl · 2021-03-21T12:45:33Z

Thanks @jkotas @SingleAccretion for the help.

jkotas

Thank you!

yowl added 4 commits February 4, 2021 20:41

attempt with LLVMMethodCodeNode -> MethodCodeNode

8410f49

attempt with LLVMMethodCodeNode -> MethodCodeNode

fdea381

build wasm32/64 clrjits and load the wasm clrjit dll for wasm32

70f98ab

Wasm64 has some macros defined, but otherwise not compiled Copied mostly from the xarch and AMD64 macros to get something to compile and run.

yowl changed the title ~~Ryujit IR -> LLVM for improved performance~~ RyuJIT IR -> LLVM for improved performance Feb 6, 2021

jkotas reviewed Feb 7, 2021

View reviewed changes

yowl added 5 commits February 7, 2021 13:05

break fix for non wasm clrjit compilation

f4827f4

wasm compilation will now complete, although has an issue with the dbg instruction in Array Resize

revert project outputpath

1025750

Remove lowering/emit/lrsa/codegen by #ifdefs

0fc5ce1

Some code alerts false where not sure what to do. Passes non-wasm tests locally.

Remove lowering/emit/lrsa/codegen by #ifdefs

8139539

Some code alerts false where not sure what to do. Passes non-wasm tests locally.

typo on #endif

13a0695

jkotas reviewed Feb 8, 2021

View reviewed changes

yowl added 3 commits February 8, 2021 14:41

remove inst* with ifdef

70750e1

remove stacklevelsetter with ifdef fix ifdef syntax

fix ifdef nesting for release config

d2075e7

fix more endif syntax for nix

ffe32da

yowl added 3 commits February 13, 2021 10:55

Dont include eeDispVars for wasm

9232ad1

exclude wasm jit from unix builds - wasm doesn't build there at the m…

e549b87

…oment attempt to add a second platform to the wasm yaml build fix problem where Array<T>.Resize has its code compiled twice.

extende jitinterface to get the LLVM module

504c2ec

yowl mentioned this pull request Feb 27, 2021

Would you consider adding libLLVM.lib to the nuget package ? dotnet/LLVMSharp#159

Closed

yowl added 4 commits February 27, 2021 15:38

Will compile but wont link (no libLLVM.lib)

bf759ae

May also need before including any LLVM header files: #undef NumItems

change llvm config path to 11 (hard coded for now)

991cbea

Tidy CMakeLists.txt for jit and add LLVM libraries

6c5d807

Fix up some merge problems

yowl added 15 commits March 12, 2021 07:09

correct force arg

1046e0e

debug the env

c6df28e

set cmakepath

03393e9

set repoRroot from $(Build.SourcesDirectory)

6e47b0d

add trailing \

f893105

remove quotes

e24ac19

build using cmake not msbuild

003c025

set up clrjit build for wasm

74a93fa

additional exports for llvm clrjit

fix cmake for undefined env var

e18bb91

Merge branch 'ryujit-llvm-imethodcodenode' into ryu-adv

fcd5a3e

# Conflicts: # src/coreclr/jit/CMakeLists.txt

remove env var

e6dd70d

ws

efbf3ff

fix for undefined

627c5b5

Merge branch 'ryujit-llvm-imethodcodenode' into ryu-adv

e3859b1

Enable CI be adding step to download and build LLVM (also need xv as …

33a1b1a

…that is the compression Llvm use) Add build of clrjit for Windows x64 Debug only. This means that WebAssembly can only be built on that combination (as is the tested case currently).

yowl force-pushed the ryujit-llvm-imethodcodenode branch from 627c5b5 to 33a1b1a Compare March 15, 2021 22:20

yowl added 2 commits March 16, 2021 09:57

remove hard coded method restriction

57f66ae

enable linking from publish some refactor to allow the signature to be retrieved to create the externals for clrjit methods.

Merge remote-tracking branch 'origin/feature/NativeAOT-LLVM' into ryu…

16a0508

…jit-llvm-imethodcodenode # Conflicts: # src/coreclr/tools/aot/ILCompiler.LLVM/CodeGen/ILToLLVMImporter.cs

yowl marked this pull request as ready for review March 16, 2021 17:27

jkotas reviewed Mar 17, 2021

View reviewed changes

yowl and others added 4 commits March 17, 2021 07:19

typo but/put

9b5c306

Co-authored-by: Jan Kotas <[email protected]>

move genType../genActualTypes to compiler.cpp

76fb9b5

Merge branch 'ryujit-llvm-imethodcodenode' of https://github.com/yowl…

c906d4a

…/runtimelab into ryujit-llvm-imethodcodenode

address feedback

e59d342

- Remove unnecessary ifdefs - Replace ifdefs with cmake exclusions - Fix passing of corinfo - remove some comments

jkotas approved these changes Mar 21, 2021

View reviewed changes

jkotas merged commit bd215e1 into dotnet:feature/NativeAOT-LLVM Mar 21, 2021

This was referenced Mar 21, 2021

Change some uses of EA_PTRSIZE to TARGET_POINTER_SIZE dotnet/runtime#49964

Closed

Move some gen... array definitions to compiler.cpp as they are declared in compiler.h/hpp dotnet/runtime#50141

Merged

		@@ -576,6 +576,10 @@ class emitter
		#elif defined(TARGET_ARM64)

NativeAOT-LLVM: RyuJIT IR -> LLVM for improved performance #647

NativeAOT-LLVM: RyuJIT IR -> LLVM for improved performance #647

Conversation

yowl commented Feb 6, 2021 • edited Loading

jkotas commented Feb 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl Feb 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkotas Feb 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl commented Feb 10, 2021

jkotas commented Feb 10, 2021

yowl commented Feb 21, 2021

jkotas commented Feb 21, 2021

yowl commented Mar 16, 2021

jkotas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl commented Mar 21, 2021

jkotas left a comment

Choose a reason for hiding this comment

yowl commented Feb 6, 2021 •

edited

Loading

yowl Feb 7, 2021 •

edited

Loading

jkotas Feb 8, 2021 •

edited

Loading