From 29d27d41746992b0657f31cd0df7349d7eef7b9a Mon Sep 17 00:00:00 2001 From: Tomas Date: Fri, 21 Feb 2020 20:38:43 +0100 Subject: [PATCH] Bail out on CORINFO_STATIC_FIELD_ADDRESS like Crossgen1 does 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 --- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) 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 0b07c472f6cbda..37f38184ba33c5 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 @@ private void getFieldInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_MET } 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 @@ private void getFieldInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_MET // // 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 @@ private void EncodeFieldBaseOffset(FieldDesc field, CORINFO_FIELD_INFO* pResult, { 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 @@ private void EncodeFieldBaseOffset(FieldDesc field, CORINFO_FIELD_INFO* pResult, 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));