From: Tomáš Rylek Date: Fri, 21 Feb 2020 22:47:15 +0000 (+0100) Subject: Bail out on CORINFO_STATIC_FIELD_ADDRESS like Crossgen1 does (#32664) X-Git-Tag: submit/tizen/20210909.063632~9557 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3a88e5ab2c303df769fb7fa98ab9ca485fcaf16d;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Bail out on CORINFO_STATIC_FIELD_ADDRESS like Crossgen1 does (#32664) This was the main problem after I overcame initial issues with composite R2R debugging and runtime initialization: for some reason, console output was weirdly distorted. By selectively compiling just a subset of the framework I bisected this into System.Console.get_Out which uses the ldsflda instruction that Crossgen1 doesn't support. We pretended to support it but did it incorrectly and so our result didn't match JIT. I have created the tracking issue https://github.com/dotnet/runtime/issues/32663 on implementing the static address as a minor perf optimization (likely only important in some corner scenarios heavily using statics). Thanks Tomas --- diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 0b07c47..37f3818 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1011,6 +1011,11 @@ namespace Internal.JitInterface } else { + if ((flags & CORINFO_ACCESS_FLAGS.CORINFO_ACCESS_ADDRESS) != 0) + { + throw new RequiresRuntimeJitException("https://github.com/dotnet/runtime/issues/32663: CORINFO_FIELD_STATIC_ADDRESS"); + } + helperId = field.HasGCStaticBase ? ReadyToRunHelperId.GetGCStaticBase : ReadyToRunHelperId.GetNonGCStaticBase; @@ -1018,13 +1023,16 @@ namespace Internal.JitInterface // // Currently, we only do this optimization for regular statics, but it // looks like it may be permissible to do this optimization for - // thread statics as well. - // + // thread statics as well. Currently there's no reason to do this + // as this code is not reachable until we implement CORINFO_FIELD_STATIC_ADDRESS + // which is something Crossgen1 doesn't do (cf. the above GitHub issue 32663). + /* if ((flags & CORINFO_ACCESS_FLAGS.CORINFO_ACCESS_ADDRESS) != 0 && (fieldAccessor != CORINFO_FIELD_ACCESSOR.CORINFO_FIELD_STATIC_TLS)) { fieldFlags |= CORINFO_FIELD_FLAGS.CORINFO_FLG_FIELD_SAFESTATIC_BYREF_RETURN; } + */ } if (!_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(field.OwningType) && @@ -2053,8 +2061,7 @@ namespace Internal.JitInterface { if (pMT.IsValueType) { - // ENCODE_CHECK_FIELD_OFFSET - // TODO: root field check import + throw new NotImplementedException("https://github.com/dotnet/runtime/issues/32630: ENCODE_CHECK_FIELD_OFFSET: root field check import"); } else { @@ -2090,6 +2097,7 @@ namespace Internal.JitInterface PreventRecursiveFieldInlinesOutsideVersionBubble(field, callerMethod); // ENCODE_FIELD_BASE_OFFSET + Debug.Assert(pResult->offset >= (uint)pMT.BaseType.InstanceByteCount.AsInt); pResult->offset -= (uint)pMT.BaseType.InstanceByteCount.AsInt; pResult->fieldAccessor = CORINFO_FIELD_ACCESSOR.CORINFO_FIELD_INSTANCE_WITH_BASE; pResult->fieldLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.FieldBaseOffset(field.OwningType));