dotnet / runtimelab

This repo is for experimentation and exploring new ideas that may or may not make it into the main dotnet/runtime repo.
MIT License
1.42k stars 199 forks source link

NativeAOT-LLVM: Merge to june 23 #2318

Closed yowl closed 1 year ago

yowl commented 1 year ago

This PR moves us forward to 23 June 23

@SingleAccretion I'm getting this error running the tests, can you recall anything upstream that might have affected this, or would waiting for https://github.com/dotnet/runtimelab/pull/2312 make sense as it seems to be about signatures:

          E:\GitHub\runtimelab\artifacts\tests\coreclr\wasi.wasm.Debug\TestWrappers\nativeaot.SmokeTests\nativeaot.SmokeTests.XUnitWrapper.cs(149,0): at nativeaot_SmokeTests._H
  elloWasm_HelloWasm_HelloWasm_._HelloWasm_HelloWasm_HelloWasm_cmd()
             at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
             at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
        Output:
EXEC : error : failed to run `E:\GitHub\runtimelab\artifacts\tests\coreclr\wasi.wasm.Debug\nativeaot\SmokeTests\HelloWasm\HelloWasm\native\HelloWasm.wasm` [E:\GitHub\runtimelab
\src\tests\build.proj]
          ?─? 1: RuntimeError: indirect call type mismatch
                     at S_P_Reflection_Execution_Internal_Reflection_Execution_MethodInvokers_InstanceMethodInvoker_RawCalliHelper__Call<System___Canon> (HelloWasm.wasm[8692]:0
  x87c349)
                     at S_P_Reflection_Execution_Internal_Reflection_Execution_MethodInvokers_InstanceMethodInvoker__CreateInstance (HelloWasm.wasm[10276]:0xa4c718)
                     at __VirtualCall_S_P_CoreLib_Internal_Reflection_Core_Execution_MethodInvoker__CreateInstance_0 (HelloWasm.wasm[16189]:0xf6e091)
                     at S_P_CoreLib_Internal_Reflection_Core_Execution_MethodInvoker__CreateInstance (HelloWasm.wasm[6815]:0x650c6d)
                     at S_P_CoreLib_System_Reflection_Runtime_MethodInfos_RuntimePlainConstructorInfo_1<S_P_CoreLib_System_Reflection_Runtime_MethodInfos_NativeFormat_NativeFor
  matMethodCommon>__Invoke (HelloWasm.wasm[13357]:0xd7f0e7)
                     at __VirtualCall_S_P_CoreLib_System_Reflection_ConstructorInfo__Invoke_0 (HelloWasm.wasm[16064]:0xf6c634)
                     at S_P_CoreLib_System_Reflection_ConstructorInfo__Invoke (HelloWasm.wasm[6134]:0x54c1c7)
                     at HelloWasm_Program__TestMetaData (HelloWasm.wasm[8831]:0x8a1f4c)
                     at HelloWasm_Program__Main (HelloWasm.wasm[5442]:0x49b79f)
                     at HelloWasm__Module___MainMethodWrapper (HelloWasm.wasm[2020]:0x11f3be)
                     at HelloWasm__Module___StartupCodeMain (HelloWasm.wasm[13786]:0xde3a7a)
                     at main (HelloWasm.wasm[24033]:0x10afdf7)
                     at __main_void (HelloWasm.wasm[24036]:0x10aff41)
                     at _start (HelloWasm.wasm[43]:0xf8cd)
SingleAccretion commented 1 year ago

@yowl w.r.t. test failures, it looks like https://github.com/dotnet/runtime/pull/86855#discussion_r1209557982.

yowl commented 1 year ago

@yowl w.r.t. test failures, it looks like dotnet/runtime#86855 (comment).

Thanks I did read that at the time, sorry.

yowl commented 1 year ago

I'm still a bit stuck on how best to solve this. In InstanceMethodInvoker I changed RawCalliHelper to

private static class RawCalliHelper
{
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static T Call<T>(delegate*<IntPtr, T> pfn, IntPtr arg)
        #if TARGET_WASM
        {
            object obj = System.Runtime.RuntimeImports.RhpRawCalli_OI((IntPtr)pfn, arg);
            return Unsafe.As<object, T>(ref obj);
        }
        #else
             => pfn(arg);
        #endif
}

But that generates this:

define ptr @"S_P_Reflection_Execution_Internal_Reflection_Execution_MethodInvokers_InstanceMethodInvoker_RawCalliHelper__Call<System___Canon>"(ptr %0, ptr %1, ptr %2, i32 %3) {
BB00:
  %4 = alloca ptr, align 4
  store ptr %1, ptr %4, align 4
  %5 = alloca ptr, align 4
  store ptr %2, ptr %5, align 4
  %6 = alloca i32, align 4
  store i32 %3, ptr %6, align 4
  %7 = alloca ptr, align 4
  store ptr null, ptr %7, align 4
  %8 = alloca ptr, align 4
  store ptr null, ptr %8, align 4
  br label %BB01

BB01:                                             ; preds = %BB00
  store ptr null, ptr %0, align 4
  br label %BB02

BB02:                                             ; preds = %BB01
  %9 = load ptr, ptr %5, align 4
  store ptr %9, ptr %7, align 4
  %10 = load i32, ptr %6, align 4
  %11 = load ptr, ptr %7, align 4
  %12 = getelementptr i8, ptr %0, i32 4
  %13 = call ptr %11(ptr %12, i32 %10)
  store ptr %13, ptr %0, align 4
  %14 = load ptr, ptr %0, align 4
  ret ptr %14
}

%13 = call ptr %11(ptr %12, i32 %10) the second parameter is i32 but this is going to end being passed to, for example:

COOP_PINVOKE_HELPER(Object *, RhpNewFinalizableAlign8, (MethodTable* pEEType))

%10 comes from the arg %3 which is i32 but that looks wrong to me. I'll see why the signature RawCalliHelper.Call is as it is.

I think this is the explanation for the current failure:

RuntimeError: null function or function signature mismatch
    at S_P_Reflection_Execution_Internal_Reflection_Execution_MethodInvokers_InstanceMethodInvoker_RawCalliHelper__Call<System___Canon> (wasm://wasm/076a667a:wasm-function[6098]:0x562c75)
SingleAccretion commented 1 year ago

It looks odd that the code is performing an indirect call, it should call the runtime import. Is S.P.Reflection.Execution compiled as a target-specific assembly, i. e. is the #if effective?

yowl commented 1 year ago

It looks odd that the code is performing an indirect call, it should call the runtime import. Is S.P.Reflection.Execution compiled as a target-specific assembly, i. e. is the #if effective?

Right, I think I just didn't compile something. Problem looks like a GC hole (if that's the right term) now. Trouble with these things are that it is difficult to be sure the problem is with the merge and wasn't just latent.

SingleAccretion commented 1 year ago

Note #2312 introduced a GC hole (but I don't think this branch is based on it).

It is suspicious that S_P_CoreLib_System_Runtime_RawCalliHelper__Call_7 is still in the call stack here. B282745 is also quite a stress test for the robustness of a conservative GC.

yowl commented 1 year ago

It is suspicious that S_P_CoreLib_System_Runtime_RawCalliHelper__Call_7 is still in the call stack here

From https://github.com/dotnet/runtimelab/blob/f390a9ca067cd0e11137a8c6a28e250ab23a3d9a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs#L234 or https://github.com/dotnet/runtimelab/blob/f390a9ca067cd0e11137a8c6a28e250ab23a3d9a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs#L259

That's ok though isn't it? Or you think there might be paths through these to unmanaged methods?

SingleAccretion commented 1 year ago

That's ok though isn't it?

Agree; it doesn't actually look relevant.

SingleAccretion commented 1 year ago

but I don't think this branch is based on it

Actually, I forgot that CI merges main before running, so #2312 is indeed in the commit list here. I have been actively looking at the issue and hope to have the fix up soon.

yowl commented 1 year ago

I forgot that CI merges main before running

Ok, great, I have to resolve some conflicts also.

yowl commented 1 year ago

Removed some tests for MD Arrays in DynamicGenerics because they start failing from https://github.com/dotnet/runtime/pull/86877

Example stack trace ``` RuntimeError: unreachable at __trap (wasm://wasm/0b0ccf12:wasm-function[37647]:0x1ac3a1d) at Module.___trap (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\D ynamicGenerics\DynamicGenerics\native\DynamicGenerics.js:6052:66) at abort (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\DynamicGen erics\DynamicGenerics\native\DynamicGenerics.js:954:3) at _abort (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\DynamicGe nerics\DynamicGenerics\native\DynamicGenerics.js:4952:7) at Assert(char const*, char const*, unsigned int, char const*) (wasm://wasm/0b0ccf12:wasm-function[49]:0x 1b768) at WKS::CObjectHeader::SetMarked() (wasm://wasm/0b0ccf12:wasm-function[1254]:0x9bb78) at WKS::gc_heap::mark_object_simple(unsigned char**) (wasm://wasm/0b0ccf12:wasm-function[1351]:0xca6b5) at WKS::GCHeap::Promote(Object**, ScanContext*, unsigned int) (wasm://wasm/0b0ccf12:wasm-function[1348]:0 xc7dbb) at GcEnumObjectsConservatively(Object**, Object**, void (*)(Object**, ScanContext*, unsigned int), ScanCo ntext*) (wasm://wasm/0b0ccf12:wasm-function[868]:0x5b341) at RedhawkGCInterface::EnumGcRefsInRegionConservatively(RtuObjectRef*, RtuObjectRef*, void*, void*) (wasm ://wasm/0b0ccf12:wasm-function[1904]:0x124876) at GcScanWasmShadowStack(void*, void*) (wasm://wasm/0b0ccf12:wasm-function[260]:0x2b40f) at Thread::GcScanRoots(void*, void*) (wasm://wasm/0b0ccf12:wasm-function[261]:0x2b47f) at GCToEEInterface::GcScanRoots(void (*)(Object**, ScanContext*, unsigned int), int, int, ScanContext*) ( wasm://wasm/0b0ccf12:wasm-function[863]:0x5acc2) at GCScan::GcScanRoots(void (*)(Object**, ScanContext*, unsigned int), int, int, ScanContext*) (wasm://wa sm/0b0ccf12:wasm-function[879]:0x5b919) at WKS::gc_heap::mark_phase(int, int) (wasm://wasm/0b0ccf12:wasm-function[1303]:0xaef12) at WKS::gc_heap::gc1() (wasm://wasm/0b0ccf12:wasm-function[1297]:0xa71bc) at WKS::gc_heap::garbage_collect(int) (wasm://wasm/0b0ccf12:wasm-function[1227]:0x92bc1) at WKS::GCHeap::GarbageCollectGeneration(unsigned int, gc_reason) (wasm://wasm/0b0ccf12:wasm-function[122 4]:0x916de) at WKS::GCHeap::GarbageCollectTry(int, int, int) (wasm://wasm/0b0ccf12:wasm-function[1625]:0x116ffb) at WKS::GCHeap::GarbageCollect(int, bool, int) (wasm://wasm/0b0ccf12:wasm-function[1624]:0x116ccf) at RhpCollect (wasm://wasm/0b0ccf12:wasm-function[1869]:0x122b2f) at S_P_CoreLib_System_Runtime_InternalCalls__RhpCollect (wasm://wasm/0b0ccf12:wasm-function[2311]:0x15acd 9) at S_P_CoreLib_System_Runtime_InternalCalls__RhCollect (wasm://wasm/0b0ccf12:wasm-function[23449]:0x16d89 de) at RhCollect (wasm://wasm/0b0ccf12:wasm-function[30823]:0x1991d84) at S_P_CoreLib_System_GC__Collect_0 (wasm://wasm/0b0ccf12:wasm-function[14455]:0xd6ea35) at DynamicGenerics_B282745_GenericType_1__test (wasm://wasm/0b0ccf12:wasm-function[11809] :0xa9e85a) at Internal_CompilerGenerated__Module___Static (wasm://wasm/0b0cc f12:wasm-function[2140]:0x139396) at S_P_CoreLib_System_Runtime_RawCalliHelper__Call_7 (wasm://wasm/0b0ccf12:wasm-function[24218]:0x1777b9a ) at S_P_CoreLib_System_Reflection_DynamicInvokeInfo__Invoke (wasm://wasm/0b0ccf12:wasm-function[2810]:0x1d e921) at S_P_Reflection_Execution_Internal_Reflection_Execution_MethodInvokers_StaticMethodInvoker__Invoke (was m://wasm/0b0ccf12:wasm-function[13452]:0xc432ca) at __VirtualCall_S_P_CoreLib_Internal_Reflection_Core_Execution_MethodInvoker__Invoke_0 (wasm://wasm/0b0c cf12:wasm-function[27227]:0x196b707) at S_P_CoreLib_Internal_Reflection_Core_Execution_MethodInvoker__Invoke (wasm://wasm/0b0ccf12:wasm-functi on[24644]:0x17d8692) at S_P_CoreLib_System_Reflection_Runtime_MethodInfos_RuntimeMethodInfo__Invoke (wasm://wasm/0b0ccf12:wasm -function[3150]:0x22f6d3) at __VirtualCall_S_P_CoreLib_System_Reflection_MethodBase__Invoke_0 (wasm://wasm/0b0ccf12:wasm-function[2 7076]:0x1969913) at S_P_CoreLib_System_Reflection_MethodBase__Invoke (wasm://wasm/0b0ccf12:wasm-function[14435]:0xd6ab77) at DynamicGenerics_B282745__testMDArrayWithPointerLikeValuesOfUnknownStructReferenceType (wasm://wasm/0b0 ccf12:wasm-function[14514]:0xd8f1f8) at DynamicGenerics_EntryPointMain___c___Main_b__0_67 (wasm://wasm/0b0ccf12:wasm-function[14513]:0xd8ebb4) at DynamicGenerics_CoreFXTestLibrary_Internal_Runner__RunTestMethod (wasm://wasm/0b0ccf12:wasm-function[1 4509]:0xd8c3c2) at DynamicGenerics_CoreFXTestLibrary_Internal_Runner__RunTest (wasm://wasm/0b0ccf12:wasm-function[14497]: 0xd85b61) at DynamicGenerics_CoreFXTestLibrary_Internal_Runner__RunTests (wasm://wasm/0b0ccf12:wasm-function[14494] :0xd814c7) at DynamicGenerics_EntryPointMain__Main (wasm://wasm/0b0ccf12:wasm-function[17268]:0x10ddf28) at DynamicGenerics__Module___MainMethodWrapper (wasm://wasm/0b0ccf12:wasm-function[2238]:0x14f224) at DynamicGenerics__Module___StartupCodeMain (wasm://wasm/0b0ccf12:wasm-function[23489]:0x16deb39) at main (wasm://wasm/0b0ccf12:wasm-function[37645]:0x1ac3914) at C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\DynamicGenerics\D ynamicGenerics\native\DynamicGenerics.js:993:22 at callMain (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\Dynamic Generics\DynamicGenerics\native\DynamicGenerics.js:6498:15) at doRun (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\DynamicGen erics\DynamicGenerics\native\DynamicGenerics.js:6552:23) at run (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\DynamicGener ics\DynamicGenerics\native\DynamicGenerics.js:6567:5) at runCaller (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\Dynami cGenerics\DynamicGenerics\native\DynamicGenerics.js:6475:19) at removeRunDependency (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTe sts\DynamicGenerics\DynamicGenerics\native\DynamicGenerics.js:919:7) at receiveInstance (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\SmokeTests\ DynamicGenerics\DynamicGenerics\native\DynamicGenerics.js:1081:5) at receiveInstantiationResult (C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\nativeaot\ SmokeTests\DynamicGenerics\DynamicGenerics\native\DynamicGenerics.js:1099:5) Node.js v18.16.1 Return code: 1 Raw output file: C:\github\runtimelab\artifacts\tests\coreclr\browser.wasm.Debug\Reports\nativeaot.Smoke Tests\DynamicGenerics\DynamicGenerics\DynamicGenerics.output.txt ```
yowl commented 1 year ago

Last commit is remoivng IsServerGC check: my understanding is Wasm won't get that as it is not multiprocessor. I also removed RawCalliHelper by mistake in a merge, so I put that back (without the Wasm workaround of course).

cc @dotnet/nativeaot-llvm as removing from draft.

jkotas commented 1 year ago

Thank you!