Wasm enable shared generics and generic virtual methods by yowl · Pull Request #7897 · dotnet/corert

yowl · 2019-12-01T02:30:48Z

Mostly taken from the cpp backend, this enables shared generics and generic virtual methods for the Wasm backend. Have tried to keep the logic as clean as I could, but maybe there's some obvious enhancements/reductions in code that I missed.

Size of HelloWasm.bc (LLVM bitcode) before 43,950KB, and after 37,593KB (these aren't exact as the after includes more tests, so actual saving is probably fractionally better) . HelloWasm before this change didn't have much in the way of generics so this mostly reflects the framework and runtime generics.

Enables the Simple/Generics tests for wasm which have the almost same coverage as cpp (Wasm includes an additional test) in terms of what is #if'ed out.

WIP as depends on #7893 and #7891

Fixes #7248

# Conflicts: # src/dirs.proj

…r at least might as well do that now rather than work around the lack of it.

# Conflicts: # tests/src/Simple/HelloWasm/Program.cs

add call for generic context need to add the hidden param where necessary.

… entries.

…ions calls incorrect.

…s when compiling.

Still have to do generic lookup functions and delegate thunks are undefined

Correct call for checking cctor to match cpp. Return the value - but its not used yet passes compilation.

Calling stub instead of signal() exception thrown: RuntimeError: index out of bounds,__ZN6EEType18get_OptionalFieldsEv@http://localhost:6931/HelloWasm.wasm:wasm-function[19519]:0xc24eb3 __ZN6EEType13get_RareFlagsEv@http://localhost:6931/HelloWasm.wasm:wasm-function[19529]:0xc257d1 __ZN6EEType14RequiresAlign8Ev@http://localhost:6931/HelloWasm.wasm:wasm-function[19615]:0xc2a3d7 _RhpNewArray@http://localhost:6931/HelloWasm.wasm:wasm-function[19617]:0xc2a523 _S_P_TypeLoader_System_Collections_Generic_LowLevelDictionary_2_System___Canon__IntPtr___Clear@http://localhost:6931/HelloWasm.wasm:wasm-function[2237]:0x2f3c6f _S_P_TypeLoader_System_Collections_Generic_LowLevelDictionary_2_System___Canon__IntPtr____ctor_0@http://localhost:6931/HelloWasm.wasm:wasm-function[1733]:0x29d8b4 _S_P_TypeLoader_System_Collections_Generic_LowLevelDictionary_2_System___Canon__IntPtr____ctor@http://localhost:6931/HelloWasm.wasm:wasm-function[1164]:0x256ca5 _S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeLoaderEnvironment___ctor@http://localhost:6931/HelloWasm.wasm:wasm-function[861]:0x22fcff _S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeLoaderEnvironment__Initialize@http://localhost:6931/HelloWasm.wasm:wasm-function[470]:0x202c0b _S_P_TypeLoader_Internal_Runtime_CompilerHelpers_LibraryInitializer__InitializeLibrary@http://localhost:6931/HelloWasm.wasm:wasm-function[400]:0x1fc3c6 _StartupCodeMain@http://localhost:6931/HelloWasm.wasm:wasm-function[396]:0x1fc2fe ___managed__Main@http://localhost:6931/HelloWasm.wasm:wasm-function[17784]:0xb875ad _main@http://localhost:6931/HelloWasm.wasm:wasm-function[19766]:0xc31ced Module._main@http://localhost:6931/HelloWasm.js:6592:33 callMain@http://localhost:6931/HelloWasm.js:7044:22 doRun@http://localhost:6931/HelloWasm.js:7102:49

Problem now seems to be that class statics are not initialised

now fails with _S_P_CoreLib_System_Diagnostics_Debug__Fail_0@http://localhost:6931/HelloWasm.wasm:wasm-function[702]:0x21d82e _S_P_CoreLib_System_Diagnostics_Debug__Assert_1@http://localhost:6931/HelloWasm.wasm:wasm-function[597]:0x20ff8a _S_P_CoreLib_System_Diagnostics_Debug__Assert@http://localhost:6931/HelloWasm.wasm:wasm-function[337]:0x1f7a56 _S_P_CoreLib_System_Threading_ManagedThreadId__AllocateId@http://localhost:6931/HelloWasm.wasm:wasm-function[2330]:0x2fb83b _S_P_CoreLib_System_Threading_ManagedThreadId___ctor@http://localhost:6931/HelloWasm.wasm:wasm-function[1682]:0x298e63 _S_P_CoreLib_System_Threading_ManagedThreadId__MakeForCurrentThread@http://localhost:6931/HelloWasm.wasm:wasm-function[1217]:0x25ae5b _S_P_CoreLib_System_Threading_ManagedThreadId__get_Current@http://localhost:6931/HelloWasm.wasm:wasm-function[810]:0x22b6ce _S_P_CoreLib_System_Runtime_CompilerServices_ClassConstructorRunner__get_CurrentManagedThreadId@http://localhost:6931/HelloWasm.wasm:wasm-function[484]:0x2050c7 _S_P_CoreLib_System_Runtime_CompilerServices_ClassConstructorRunner__EnsureClassConstructorRun@http://localhost:6931/HelloWasm.wasm:wasm-function[391]:0x1fbf2d

for the generic dict, try to remove the +1 pointer as otherwise the index lookup is one off - this maybe the wrong way to fix this.

Fails as InternalGetResourceString throws.

…erOffset is not working so the calli fails.

MichalStrehovsky

I started looking but I haven't looked at the two files with the biggest changes yet. I hope I'll get to those soon. Sorry for the delays.

src/Common/src/Internal/NativeFormat/NativeFormatWriter.cs

src/Common/src/Internal/Runtime/RuntimeConstants.cs

...mbly/src/Compiler/DependencyAnalysis/WebAssemblyReadyToRunGenericLookupFromDictionaryNode.cs

src/ILCompiler.WebAssembly/src/Compiler/DependencyAnalysis/WebAssemblyVTableSlotNode.cs

MichalStrehovsky · 2019-12-13T19:18:12Z

tests/src/Simple/Generics/Generics.csproj

@@ -1,7 +1,15 @@
-<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="Current">


I think all changes to this file can be backed out.

MichalStrehovsky · 2019-12-13T19:18:40Z

tests/src/Simple/HelloWasm/HelloWasm.csproj


 	<ProjectReference Include="ILHelpers.ilproj" Condition="'$(OS)' == 'Windows_NT'" />
    <IlcArg Include="-r:$(IntermediateOutputPath)\ILHelpers.dll" Condition="'$(OS)' == 'Windows_NT'" />
+    <IlcArg Include="--usesharedgenerics" />


Should not be needed since shared genercis are now the default for wasm too.

having a problem with this. If its not here, how does it get into the .rsp?

I wasn't reading

corert/src/ILCompiler/src/Program.cs

Lines 314 to 315 in ac04886

SharedGenericsMode genericsMode = _useSharedGenerics || !_isWasmCodegen ?

SharedGenericsMode.CanonicalReferenceTypes : SharedGenericsMode.Disabled;

correctly. Once Wasm enables shared generics, this will always be true, so how far should I go with this logic, just set useSharedGenerics to default true with a comment "all current backends require shared generics" and warn if the command line arg is set false, or completely remove the option to turn off shared generics, the command line arg, and the SharedGenericsMode enum?

Could you remove this?

_S_P_CoreLib_System_Diagnostics_Debug__Assert@http://localhost:6931/Generics.wasm:wasm-function[375]:0x21682b _S_P_TypeLoader_Internal_TypeSystem_TypeDesc__SetRuntimeTypeHandleUnsafe@http://localhost:6931/Generics.wasm:wasm-function[11211]:0x78026a _S_P_TypeLoader_Internal_TypeSystem_CanonType__Initialize@http://localhost:6931/Generics.wasm:wasm-function[13993]:0x8fedfe _S_P_TypeLoader_Internal_TypeSystem_CanonType___ctor@http://localhost:6931/Generics.wasm:wasm-function[13024]:0x88a3cf _S_P_TypeLoader_Internal_TypeSystem_TypeSystemContext__get_CanonType@http://localhost:6931/Generics.wasm:wasm-function[11209]:0x78010c _S_P_TypeLoader_Internal_TypeSystem_TypeSystemContext__ResolveRuntimeTypeHandle@http://localhost:6931/Generics.wasm:wasm-function[10335]:0x71

…red-fails # Conflicts: # src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs # tests/src/Simple/HelloWasm/Program.cs

yowl · 2019-12-14T18:15:51Z

Thanks, have made the adjustment for the fat function offset

yowl · 2019-12-15T17:26:07Z

Oh dear, I seem to have broken something, let me look at that.

…red-fails # Conflicts: # src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs # tests/src/Simple/HelloWasm/Program.cs

yowl · 2019-12-26T20:24:40Z

Remove WIP as dependencies now merged

yowl · 2019-12-27T03:06:14Z

Noticed a problem:
With this IL

  IL_005B:  constrained valuetype TKey_System.Collections.Generic.KeyValuePair`2<class System.__Canon, class System.__Canon>
  IL_0061:  callvirt    instance int32 System.Object::GetHashCode()

Which is from the method

// [S.P.CoreLib]System.Collections.Generic.Dictionary`2<TKey_System.Collections.Generic.KeyValuePair`2<System.__Canon,System.__Canon>,TValue_System.__Canon>.FindValue(KeyValuePair`2<__Canon,__Canon>)
.method instance class TValue_System.__Canon& FindValue(valuetype TKey_System.Collections.Generic.KeyValuePair`2<class System.__Canon, class System.__Canon>) cil managed

When getting the constrained type we use

        private void ImportConstrainedPrefix(int token)
        {
            _constrainedType = (TypeDesc)_methodIL.GetObject(token);
        }

Which results in _constrainedType being a RuntimeDeterminedType. This is a problem in the callvirt as we attempt to get the target method using

targetMethod = constrainedType.TryResolveConstraintMethodApprox(canonMethod.OwningType, canonMethod, out _);

which expects a MetadataType at

corert/src/Common/src/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs

Line 179 in a3f2141

    
           return FindVirtualFunctionTargetMethodOnObjectType(targetMethod, (MetadataType)objectType);

.

Where have I gone wrong here, should I even be creating code for TKey, if that's a value type, then shouldn't it be the actual type?

MichalStrehovsky · 2019-12-28T13:22:04Z

Which results in _constrainedType being a RuntimeDeterminedType. This is a problem in the callvirt as we attempt to get the target method using

I would just do what the IL scanner is doing: grab the canonical form of the constrained type:

corert/src/ILCompiler.Compiler/src/IL/ILImporter.Scanner.cs

Lines 420 to 422 in 6c06857

    
           TypeDesc constrained = _constrained; 
        
           if (constrained.IsRuntimeDeterminedSubtype) 
        
               constrained = constrained.ConvertToCanonForm(CanonicalFormKind.Specific);

The scanner keeps both canonical and runtime determined form of the method, and uses the canonical for things like this (TryResolveConstraintMethodApprox is called with canonical method and canonical constraint). The runtime determined form is only needed if we will have to do a generic lookup, but constrained calls don't end up needing that because they acquire the generic context from this.

Note that the scanner still uses the runtime determined form of the constraint in case we need to box.

yowl · 2019-12-28T14:19:03Z

Moving back to WIP as

        var dict = new Dictionary<KeyValuePair<string, string>, string>();
        var notContainsKey = dict.ContainsKey(new KeyValuePair<string, string>("", ""));
        dict.Add(new KeyValuePair<string, string>("", ""), "");
        var containsKey = dict.ContainsKey(new KeyValuePair<string, string>("", ""));

is failing on the Add with

exception thrown: RuntimeError: index out of bounds,_S_P_CoreLib_Internal_Runtime_EEType__get_Kind@http://localhost:6931/HelloWasm.wasm:wasm-function[632]:0x204237
_S_P_CoreLib_Internal_Runtime_EEType__get_IsParameterizedType@http://localhost:6931/HelloWasm.wasm:wasm-function[266]:0x1e65f6
_S_P_CoreLib_Internal_Runtime_EEType__get_IsPointerType@http://localhost:6931/HelloWasm.wasm:wasm-function[1424]:0x262ed0
_S_P_CoreLib_System_EETypePtr__get_IsPointer@http://localhost:6931/HelloWasm.wasm:wasm-function[1025]:0x2324e7
_S_P_CoreLib_System_ValueType__RegularGetValueTypeHashCode@http://localhost:6931/HelloWasm.wasm:wasm-function[3190]:0x33b9eb
_S_P_CoreLib_System_ValueType__GetHashCodeImpl@http://localhost:6931/HelloWasm.wasm:wasm-function[2267]:0x2c0903
_S_P_CoreLib_System_ValueType__GetHashCode@http://localhost:6931/HelloWasm.wasm:wasm-function[2266]:0x2c0774
_S_P_CoreLib_System_Collections_Generic_Dictionary_2_S_P_CoreLib_System_Collections_Generic_KeyValuePair_2_System___Canon__System___Canon___System___Canon___TryInsert@http://localhost:6931/HelloWasm.wasm:wasm-function[2011]:0x2a62fb
_S_P_CoreLib_System_Collections_Generic_Dictionary_2_S_P_CoreLib_System_Collections_Generic_KeyValuePair_2_System___Canon__System___Canon___System___Canon___Add@http://localhost:6931/HelloWasm.wasm:wasm-function[1050]:0x233e4d
_HelloWasm_Program__TestConstrainedValueTypeCallVirt@http://localhost:6931/HelloWasm.wasm:wasm-function[980]:0x22adca

yowl · 2019-12-29T20:22:04Z

Thanks for the constrained info. For the ValueType.GetHashCode problem, I think I have the special unboxing thunks ok as this code is fine

    private static void TestCallValueTypeGetHashCodeFromGeneric()
    {
        var x = new GenStruct<string>("").GetHashCode();
    }

    public struct GenStruct<TKey>
    {
        private TKey key; 

        public GenStruct(TKey key)
        {
            this.key = key;
        }
    }

However when the GetHashCode is called from within a generic method :

    private static void TestCallValueTypeGetHashCodeFromGeneric()
    {
        new GetHashCodeCaller<GenStruct<string>, string>().CallValueTypeGetHashCodeFromGeneric(new GenStruct<string>(""));
    }

    public class GetHashCodeCaller<TKey, TValue>
    {
        public void CallValueTypeGetHashCodeFromGeneric(TKey k)
        {
            k.GetHashCode();
        }
    }

Then I get this error, or rather as I'm not using Dict<,> anymore:

RuntimeError: index out of bounds,
___GenericLookupFromDict_S_P_CoreLib_System_EETypePtr__EETypePtrOf_System___Canon__TypeHandle_T_System___Canon@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[502]:0x1f84eb
_S_P_CoreLib_System_EETypePtr__EETypePtrOf_System___Canon_@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[417]:0x1f13f1
_HelloWasm_Program_GenStruct_1_System___Canon_____GetFieldHelper@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[3863]:0x35f5db
__Boxed_HelloWasm_Program_GenStruct_1_System___Canon____unbox_HelloWasm_Program_GenStruct_1____GetFieldHelper@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[3862]:0x35f57d
_S_P_CoreLib_System_ValueType__RegularGetValueTypeHashCode@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[3187]:0x338a7c
_S_P_CoreLib_System_ValueType__GetHashCodeImpl@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[2263]:0x2bd9b3
_S_P_CoreLib_System_ValueType__GetHashCode@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[2262]:0x2bd824
_HelloWasm_Program_GetHashCodeCaller_2_HelloWasm_Program_GenStruct_1_System___Canon___System___Canon___CallValueTypeGetHashCodeFromGeneric@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[1050]:0x233c89
_HelloWasm_Program__TestCallValueTypeGetHashCodeFromGeneric@file:///E:/GitHub/corert/tests/src/Simple/HelloWasm/bin/Debug/wasm/native/HelloWasm.wasm:wasm-function[981]:0x22adde

I want to ask if I have the RhBox correct. For the working first case, the RhBox is called with the concrete type, __EEType_HelloWasm_Program_GenStruct_1<String>___SYMBOL, but when called from the generic method, the type passed to RhBox is __EEType_HelloWasm_Program_GenStruct_1<System___Canon>___SYMBOL because the logic for doing a generic dict lookup is

            TypeDesc type = (TypeDesc)_methodIL.GetObject(token);
...
            if (type.IsRuntimeDeterminedSubtype)

And that is false. Is this wrong?

…ooked up type.

yowl · 2019-12-30T00:25:34Z

My mistake, was looking the the ILOpcode box, not the callvirt where I didn't have the lookup code. Its in now along with a test. Removing WIP. Sorry about the back and forth on this. I have the standard tests working, but then move on to trying the Uno Platform where more errors are uncovered.

MichalStrehovsky

Thanks! Once this feedback is addressed, we can check this in.

MichalStrehovsky · 2019-12-31T06:58:09Z

src/ILCompiler.CppCodeGen/src/CppCodeGen/ILToCppImporter.cs

                Append(gvmSlotVarName);
                Append(" & ");
-                Append(FatFunctionPointerConstants.Offset.ToString());
+                Append(method.Context.Target.FatFunctionPointerOffset.ToString());


Just a nit (also elsewhere):

Suggested change

Append(method.Context.Target.FatFunctionPointerOffset.ToString());

Append(_typeSystemContext.Target.FatFunctionPointerOffset.ToString());

MichalStrehovsky · 2019-12-31T07:01:33Z

src/ILCompiler.WebAssembly/src/Compiler/DependencyAnalysis/WebAssemblyUnboxingThunkNode.cs

@@ -0,0 +1,34 @@
+using System;


Could you add copyright header?

MichalStrehovsky · 2019-12-31T07:02:41Z

src/ILCompiler.WebAssembly/src/Compiler/DependencyAnalysis/WebAssemblyUnboxingThunkNode.cs

@@ -0,0 +1,34 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;


Nit: is this used?

MichalStrehovsky · 2019-12-31T07:04:28Z

src/ILCompiler/src/Program.cs


-            SharedGenericsMode genericsMode = _useSharedGenerics || !_isWasmCodegen ?
-                SharedGenericsMode.CanonicalReferenceTypes : SharedGenericsMode.Disabled;
+            SharedGenericsMode genericsMode = SharedGenericsMode.CanonicalReferenceTypes;


_useSharedGenerics is now unused. Let's delete the command line option and field.

MichalStrehovsky · 2019-12-31T07:06:27Z

tests/src/Simple/Generics/Generics.cs

        TestReflectionInvoke.Run();
        TestFieldAccess.Run();
        TestDevirtualization.Run();
-        TestGenericInlining.Run();


Was this lost in a rebase?

Yes, reinstated.

MichalStrehovsky · 2019-12-31T07:13:08Z

src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs

+        /// Offset by which fat function pointers are shifted to distinguish them
+        /// from real function pointers.  Keep in line with FatFunctionPointerConstants
+        /// </summary> 
+        internal const uint FatFunctionPointerOffset = (uint)1 << 31;


Could you switch this to the one obtained from Context.Target?

Done. Moved a helper function from static to instance to get access.

MichalStrehovsky · 2019-12-31T07:20:00Z

src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs

+                //TODO refactor argument creation with else below
+                arguments = new StackEntry[]
+                            {
+                                new ExpressionEntry(StackValueKind.ValueType, "eeType", CallGenericHelper(ReadyToRunHelperId.TypeHandle, type),


This would probably fix the TestNullableCasting test in the Simple\Generics test:

Suggested change

new ExpressionEntry(StackValueKind.ValueType, "eeType", CallGenericHelper(ReadyToRunHelperId.TypeHandle, type),

new ExpressionEntry(StackValueKind.ValueType, "eeType", CallGenericHelper(ReadyToRunHelperId.TypeHandleForCasting, type),

If it doesn't fix it, then leave this as-is for now. Let's not add more scope to this.

Unfortunately hits

corert/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs

Line 48 in 7cb8f8c

Debug.Assert(factory.GenericLookup.Type((TypeDesc)target) != factory.GenericLookup.UnwrapNullableType((TypeDesc)target));

with target {[S.P.CoreLib]System.Collections.Generic.ICollection1<T_System.__Canon>}` Have opened #7934

MichalStrehovsky · 2019-12-31T07:26:47Z

src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs

-            HandleCall(null, methodSignature, ILOpcode.calli, calliTarget: ((ExpressionEntry)_stack.Pop()).ValueAsType(LLVM.PointerType(GetLLVMSignatureForMethod(methodSignature), 0), _builder));
+            MethodSignature methodSignature = (MethodSignature)_canonMethodIL.GetObject(token);
+
+            if (_method.ToString().Contains("InvokeRet"))


Can be deleted?

MichalStrehovsky · 2019-12-31T07:30:39Z

src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs

+                }
+                else
+                {
+                    if (ConstructedEETypeNode.CreationAllowed(typeDesc))


Do we need this if check? If we call GetEETypePointerForTypeDesc with true below, it should do the right thing wrt dependency tracking.

No, works with it removed.

MichalStrehovsky · 2019-12-31T07:33:15Z

src/ILCompiler.WebAssembly/src/CodeGen/WebAssemblyObjectWriter.cs

                    if (node == null)
                        continue;
-
+                    


Nit: undo white space change (same below).

yowl

Thanks for checking

yowl · 2019-12-01T02:54:27Z

src/Common/src/Internal/Runtime/RuntimeConstants.cs

        /// </summary>
+#if WASM
+        // WebAssembly uses index tables, not addresses for function pointers.  This is going to be contentious it is baked into Ilc and hence an Ilc for x64 will not be able to compile Wasm and vice versa.  Alternative ways round this?.
+        public const int Offset = 1 << 31;


It was mentioned #7248 (comment) that there might be other better solutions available for Wasm.

...mbly/src/Compiler/DependencyAnalysis/WebAssemblyReadyToRunGenericLookupFromDictionaryNode.cs

yowl · 2019-12-14T18:03:54Z

tests/src/Simple/HelloWasm/HelloWasm.csproj


 	<ProjectReference Include="ILHelpers.ilproj" Condition="'$(OS)' == 'Windows_NT'" />
    <IlcArg Include="-r:$(IntermediateOutputPath)\ILHelpers.dll" Condition="'$(OS)' == 'Windows_NT'" />
+    <IlcArg Include="--usesharedgenerics" />


having a problem with this. If its not here, how does it get into the .rsp?

yowl · 2019-12-16T13:55:04Z

tests/src/Simple/HelloWasm/HelloWasm.csproj


 	<ProjectReference Include="ILHelpers.ilproj" Condition="'$(OS)' == 'Windows_NT'" />
    <IlcArg Include="-r:$(IntermediateOutputPath)\ILHelpers.dll" Condition="'$(OS)' == 'Windows_NT'" />
+    <IlcArg Include="--usesharedgenerics" />


I wasn't reading

corert/src/ILCompiler/src/Program.cs

Lines 314 to 315 in ac04886

SharedGenericsMode genericsMode = _useSharedGenerics || !_isWasmCodegen ?

SharedGenericsMode.CanonicalReferenceTypes : SharedGenericsMode.Disabled;

correctly. Once Wasm enables shared generics, this will always be true, so how far should I go with this logic, just set useSharedGenerics to default true with a comment "all current backends require shared generics" and warn if the command line arg is set false, or completely remove the option to turn off shared generics, the command line arg, and the SharedGenericsMode enum?

yowl · 2019-12-31T19:46:49Z

src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs

+                //TODO refactor argument creation with else below
+                arguments = new StackEntry[]
+                            {
+                                new ExpressionEntry(StackValueKind.ValueType, "eeType", CallGenericHelper(ReadyToRunHelperId.TypeHandle, type),


Unfortunately hits

corert/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs

Line 48 in 7cb8f8c

Debug.Assert(factory.GenericLookup.Type((TypeDesc)target) != factory.GenericLookup.UnwrapNullableType((TypeDesc)target));

with target {[S.P.CoreLib]System.Collections.Generic.ICollection1<T_System.__Canon>}` Have opened #7934

yowl · 2019-12-31T22:27:19Z

src/ILCompiler.WebAssembly/src/Compiler/DependencyAnalysis/WebAssemblyUnboxingThunkNode.cs

@@ -0,0 +1,34 @@
+using System;


yowl · 2019-12-31T22:27:29Z

src/ILCompiler/src/Program.cs


-            SharedGenericsMode genericsMode = _useSharedGenerics || !_isWasmCodegen ?
-                SharedGenericsMode.CanonicalReferenceTypes : SharedGenericsMode.Disabled;
+            SharedGenericsMode genericsMode = SharedGenericsMode.CanonicalReferenceTypes;


yowl · 2019-12-31T22:27:43Z

tests/src/Simple/Generics/Generics.cs

        TestReflectionInvoke.Run();
        TestFieldAccess.Run();
        TestDevirtualization.Run();
-        TestGenericInlining.Run();


Yes, reinstated.

yowl · 2019-12-31T22:27:56Z

tests/src/Simple/Generics/Generics.csproj

 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <ItemGroup>
    <Compile Include="*.cs" />
+    <IlcArg Include="--usesharedgenerics" />


yowl · 2019-12-31T22:28:09Z

tests/src/Simple/HelloWasm/HelloWasm.csproj


 	<ProjectReference Include="ILHelpers.ilproj" Condition="'$(OS)' == 'Windows_NT'" />
    <IlcArg Include="-r:$(IntermediateOutputPath)\ILHelpers.dll" Condition="'$(OS)' == 'Windows_NT'" />
+    <IlcArg Include="--usesharedgenerics" />


MichalStrehovsky

Thanks for your patience on this!

yowl added 30 commits April 20, 2019 12:42

start to bring across cpp implementation of gvm/shared generics

48b8063

merge

ff20b77

Merge remote-tracking branch 'master/master' into wasm-gvm

76d8f95

# Conflicts: # src/dirs.proj

add test

3bec953

some debugging code, stopped here as we need a GCStaticsNode first, o…

a4e0ca1

…r at least might as well do that now rather than work around the lack of it.

Merge branch 'master' into wasm-gvm

4819816

# Conflicts: # tests/src/Simple/HelloWasm/Program.cs

stub out some places where a lookup is needed.

b5dd820

would get to the end of compiling but for the GVM dispatch path

59f08ee

add first attempt at hooking GVM - does not run.

3072de8

add simpler test

24636d8

add call for generic context need to add the hidden param where necessary.

add hidden param to llvm signature

7b8ac4a

compilation does not finish as markedstack and _deferred keep getting…

4a1819e

… entries.

does compile at least until verify, but takes a long time. Some funct…

8a51ab3

…ions calls incorrect.

fill in some blanks to remove the first llvm verify errors

62df076

creates a valid llvm

15cf843

change store to use canon form for cast. Remove most thrown exception…

d5e40e2

…s when compiling.

fixed some calls to ldftn for FatFunctionPointer path .

ea527cd

Still have to do generic lookup functions and delegate thunks are undefined

most delegate thunks are now correctly named

f03c0f0

add stub for generic helper

9b077ee

fix type casts for pointers.

e1e55de

Correct call for checking cctor to match cpp. Return the value - but its not used yet passes compilation.

do not derefence the generic look up - copy/paste error

793d8f2

Problem now seems to be that class statics are not initialised

remove hidden param from shadow stack counting

861f482

some pointer arithmetic fixes

82c3541

for the generic dict, try to remove the +1 pointer as otherwise the index lookup is one off - this maybe the wrong way to fix this.

fix dict lookup from type

192f437

fixes some gen lookup stuff - runs for a while at least

e0daf2b

gets into the Program Main.

a6e6869

Fails as InternalGetResourceString throws.

I believe this is getting the right pointer, but fat FatFunctionPoint…

fa5b47f

…erOffset is not working so the calli fails.

half calli hidden

cb16438

MichalStrehovsky reviewed Dec 13, 2019

View reviewed changes

yowl added 5 commits December 14, 2019 10:37

back out some unwanted csproj changes

59fc738

Address some feedback

610afe8

remove blank line

0f4de2c

Merge remote-tracking branch 'corert/master' into RunReferenceTypeSha…

5551668

…red-fails # Conflicts: # src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs # tests/src/Simple/HelloWasm/Program.cs

yowl added 2 commits December 16, 2019 09:26

temporary remove logic pending feedback

67c8530

Merge remote-tracking branch 'corert/master' into RunReferenceTypeSha…

10c605d

…red-fails # Conflicts: # src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs # tests/src/Simple/HelloWasm/Program.cs

yowl changed the title ~~WIP: Wasm enable shared generics and generic virtual methods~~ Wasm enable shared generics and generic virtual methods Dec 26, 2019

yowl added 3 commits December 27, 2019 13:24

Fails

841d63a

add test like HttpHeaders GetValuesAsStrings

b295a92

add test for storing generic call return value

cee4c80

constrained callvirt fix

d5b42d7

yowl changed the title ~~Wasm enable shared generics and generic virtual methods~~ WIP:Wasm enable shared generics and generic virtual methods Dec 28, 2019

Add test and code for callvirt on valuetypes to box to the gen dict l…

d5534b3

…ooked up type.

yowl changed the title ~~WIP:Wasm enable shared generics and generic virtual methods~~ Wasm enable shared generics and generic virtual methods Dec 30, 2019

MichalStrehovsky reviewed Dec 31, 2019

View reviewed changes

address feedback

a8f238a

yowl commented Dec 31, 2019

View reviewed changes

MichalStrehovsky approved these changes Jan 2, 2020

View reviewed changes

MichalStrehovsky merged commit f52a8e5 into dotnet:master Jan 2, 2020

yowl deleted the RunReferenceTypeShared-fails branch January 3, 2020 19:50

		@@ -1,7 +1,15 @@
		<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
		<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="Current">

	SharedGenericsMode genericsMode = _useSharedGenerics \|\| !_isWasmCodegen ?
	SharedGenericsMode.CanonicalReferenceTypes : SharedGenericsMode.Disabled;

	Append(method.Context.Target.FatFunctionPointerOffset.ToString());
	Append(_typeSystemContext.Target.FatFunctionPointerOffset.ToString());

	new ExpressionEntry(StackValueKind.ValueType, "eeType", CallGenericHelper(ReadyToRunHelperId.TypeHandle, type),
	new ExpressionEntry(StackValueKind.ValueType, "eeType", CallGenericHelper(ReadyToRunHelperId.TypeHandleForCasting, type),

Conversation

yowl commented Dec 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichalStrehovsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yowl commented Dec 14, 2019

Uh oh!

yowl commented Dec 15, 2019

Uh oh!

yowl commented Dec 26, 2019

Uh oh!

yowl commented Dec 27, 2019

Uh oh!

MichalStrehovsky commented Dec 28, 2019

Uh oh!

yowl commented Dec 28, 2019

Uh oh!

yowl commented Dec 29, 2019

Uh oh!

yowl commented Dec 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichalStrehovsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

yowl commented Dec 1, 2019 •

edited

Loading

yowl commented Dec 30, 2019 •

edited

Loading