Fix IL interpreter to run on Hello World (#37825)

* Fixed type cast compilation errors

* Temporary changes to trigger the IL interpreter

* Added lock holder when invoking JIT from interpreter

* Added get_IsSupported as an intrinsic

* Fixed widening of narrow integral types

* Added COMPlus_InterpreterDebuggable flag to trigger the IL interpreter in the Debug/Checked configurations

* Added more accurate description of InterpreterJITThreshold

* Fixed assertion failure in AsyncPromoteToTier1

* Renamed env var to ForceInterpreterTier0

* Fixed a bug in AsyncPromoteToTier1

* Fixed the correct semantics of the env var ForceInterpreterTier0

* Added env var ForceInterpreterAlways

* Turned off FEATURE_INTERPRETER

* Moved CodeVersionManager::LockHolder to minimize scope

* Removed redundant check for ForceInterpreterAlways

* Renamed COMPlus_ env vars to ForceJitOptimization and ForceInterpreter

* Fixed a pre-existing bug for async promotion to tier 1

* Removed redundant annotations on CLR config values

* Modified handling of get_IsSupported as a JIT intrinsic

* This does not seems a correct fix for the pre-existing promotion bug

* Added call to ScheduleBackgroundWork after AsyncPromoteToTier1

* Added more precise check for hardware intrinsics in interpreter

* Converted C-style type casts to static_cast<...>

* Renamed ForceJitOptimization to EnableJitOptimization

* Added COMPlus environment variable to force get_IsSupported to return false for HW intrinsics

* Update src/coreclr/src/vm/interpreter.cpp

Co-authored-by: John Salem <josalem@microsoft.com>
* Removed #ifdef PAL_STDCPP_COMPAT when including 'type_traits'

* Removed EnableJitOptimization variable

Co-authored-by: John Salem <josalem@microsoft.com>

diff --git a/src/coreclr/src/inc/clrconfigvalues.h b/src/coreclr/src/inc/clrconfigvalues.h
index 267dd08..58304f0 100644 (file)
--- a/src/coreclr/src/inc/clrconfigvalues.h
+++ b/src/coreclr/src/inc/clrconfigvalues.h
@@ -395,6 +395,8 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_TraceInterpreterIL, W("TraceInterpreterIL"), 0
 RETAIL_CONFIG_DWORD_INFO(INTERNAL_TraceInterpreterOstack, W("TraceInterpreterOstack"), 0, "Logs operand stack after each IL instruction of interpreted methods to the console")
 CONFIG_DWORD_INFO(INTERNAL_TraceInterpreterVerbose, W("TraceInterpreterVerbose"), 0, "Logs interpreter progress with detailed messages to the console")
 CONFIG_DWORD_INFO(INTERNAL_TraceInterpreterJITTransition, W("TraceInterpreterJITTransition"), 0, "Logs when the interpreter determines a method should be JITted")
+RETAIL_CONFIG_DWORD_INFO(INTERNAL_ForceInterpreter, W("ForceInterpreter"), 0, "If non-zero, force the interpreter to be used")
+RETAIL_CONFIG_DWORD_INFO(INTERNAL_InterpreterHWIntrinsicsIsSupportedFalse, W("InterpreterHWIntrinsicsIsSupportedFalse"), 0, "If non-zero, force get_IsSupported to return false for hardware intrinsics") // for internal testing purposes
 #endif
 // The JIT queries this ConfigDWORD but it doesn't know if FEATURE_INTERPRETER is enabled
 RETAIL_CONFIG_DWORD_INFO(INTERNAL_InterpreterFallback, W("InterpreterFallback"), 0, "Fallback to the interpreter when the JIT compiler fails")

diff --git a/src/coreclr/src/vm/interpreter.cpp b/src/coreclr/src/vm/interpreter.cpp
index a25c334..bfd840d 100644 (file)
--- a/src/coreclr/src/vm/interpreter.cpp
+++ b/src/coreclr/src/vm/interpreter.cpp
@@ -279,7 +279,7 @@ void InterpreterMethodInfo::InitArgInfo(CEEInfo* comp, CORINFO_METHOD_INFO* meth
                 }
                 m_argDescs[k].m_typeStackNormal = m_argDescs[k].m_type;
                 m_argDescs[k].m_nativeOffset = argOffsets_[k];
-                m_argDescs[k].m_directOffset = reinterpret_cast<short>(ArgSlotEndianessFixup(directOffset, sizeof(void*)));
+                m_argDescs[k].m_directOffset = static_cast<short>(reinterpret_cast<intptr_t>(ArgSlotEndianessFixup(directOffset, sizeof(void*))));
                 directOffset++;
                 k++;
             }
@@ -302,18 +302,18 @@ void InterpreterMethodInfo::InitArgInfo(CEEInfo* comp, CORINFO_METHOD_INFO* meth
 #endif // defined(HOST_ARM)
                 )
             {
-                directRetBuffOffset = reinterpret_cast<short>(ArgSlotEndianessFixup(directOffset, sizeof(void*)));
+                directRetBuffOffset = static_cast<short>(reinterpret_cast<intptr_t>(ArgSlotEndianessFixup(directOffset, sizeof(void*))));
                 directOffset++;
             }
 #if defined(HOST_AMD64)
             if (GetFlag<Flag_isVarArg>())
             {
-                directVarArgOffset = reinterpret_cast<short>(ArgSlotEndianessFixup(directOffset, sizeof(void*)));
+                directVarArgOffset = static_cast<short>(reinterpret_cast<intptr_t>(ArgSlotEndianessFixup(directOffset, sizeof(void*))));
                 directOffset++;
             }
             if (GetFlag<Flag_hasGenericsContextArg>())
             {
-                directTypeParamOffset = reinterpret_cast<short>(ArgSlotEndianessFixup(directOffset, sizeof(void*)));
+                directTypeParamOffset = static_cast<short>(reinterpret_cast<intptr_t>(ArgSlotEndianessFixup(directOffset, sizeof(void*))));
                 directOffset++;
             }
 #endif
@@ -343,11 +343,11 @@ void InterpreterMethodInfo::InitArgInfo(CEEInfo* comp, CORINFO_METHOD_INFO* meth
                 // When invoking the interpreter directly, large value types are always passed by reference.
                 if (it.IsLargeStruct(comp))
                 {
-                    m_argDescs[k].m_directOffset = reinterpret_cast<short>(ArgSlotEndianessFixup(directOffset, sizeof(void*)));
+                    m_argDescs[k].m_directOffset = static_cast<short>(reinterpret_cast<intptr_t>(ArgSlotEndianessFixup(directOffset, sizeof(void*))));
                 }
                 else
                 {
-                    m_argDescs[k].m_directOffset = reinterpret_cast<short>(ArgSlotEndianessFixup(directOffset, it.Size(comp)));
+                    m_argDescs[k].m_directOffset = static_cast<short>(reinterpret_cast<intptr_t>(ArgSlotEndianessFixup(directOffset, it.Size(comp))));
                 }
                 argPtr = comp->getArgNext(argPtr);
                 directOffset++;
@@ -1627,6 +1627,8 @@ void Interpreter::JitMethodIfAppropriate(InterpreterMethodInfo* interpMethInfo,
     } CONTRACTL_END;
 
     unsigned int MaxInterpretCount = s_InterpreterJITThreshold.val(CLRConfig::INTERNAL_InterpreterJITThreshold);
+    bool scheduleTieringBackgroundWork = false;
+    TieredCompilationManager *tieredCompilationManager = GetAppDomain()->GetTieredCompilationManager();
 
     if (force || interpMethInfo->m_invocations > MaxInterpretCount)
     {
@@ -1663,14 +1665,24 @@ void Interpreter::JitMethodIfAppropriate(InterpreterMethodInfo* interpMethInfo,
             // interpreter I didn't wring my hands too much trying to determine the ideal
             // policy.
 #ifdef FEATURE_TIERED_COMPILATION
-            bool scheduleTieringBackgroundWork = false;
+            CodeVersionManager::LockHolder _lockHolder;
             NativeCodeVersion activeCodeVersion = md->GetCodeVersionManager()->GetActiveILCodeVersion(md).GetActiveNativeCodeVersion(md);
-            GetAppDomain()->GetTieredCompilationManager()->AsyncPromoteToTier1(activeCodeVersion, &scheduleTieringBackgroundWork);
+            ILCodeVersion ilCodeVersion = activeCodeVersion.GetILCodeVersion();
+            if (activeCodeVersion.GetOptimizationTier() == NativeCodeVersion::OptimizationTier0 &&
+                !ilCodeVersion.HasAnyOptimizedNativeCodeVersion(activeCodeVersion))
+            {
+                tieredCompilationManager->AsyncPromoteToTier1(activeCodeVersion, &scheduleTieringBackgroundWork);
+            }
 #else
 #error FEATURE_INTERPRETER depends on FEATURE_TIERED_COMPILATION now
 #endif
         }
     }
+
+    if (scheduleTieringBackgroundWork)
+    {
+        tieredCompilationManager->ScheduleBackgroundWork(); // requires GC_TRIGGERS
+    }
 }
 
 // static
@@ -7887,16 +7899,40 @@ void Interpreter::LdElemWithType()
         }
         else
         {
-            T res = reinterpret_cast<Array<T>*>(a)->GetDirectConstPointerToNonObjectElements()[index];
+            intptr_t res_ptr = reinterpret_cast<intptr_t>(reinterpret_cast<Array<T>*>(a)->GetDirectConstPointerToNonObjectElements());
             if (cit == CORINFO_TYPE_INT)
             {
+                assert(std::is_integral<T>::value);
+
                 // Widen narrow types.
-                int ires = (int)res;
+                int ires;
+                switch (sizeof(T))
+                {
+                case 1:
+                    ires = std::is_same<T, INT8>::value ?
+                           static_cast<int>(reinterpret_cast<INT8*>(res_ptr)[index]) :
+                           static_cast<int>(reinterpret_cast<UINT8*>(res_ptr)[index]);
+                    break;
+                case 2:
+                    ires = std::is_same<T, INT16>::value ?
+                           static_cast<int>(reinterpret_cast<INT16*>(res_ptr)[index]) :
+                           static_cast<int>(reinterpret_cast<UINT16*>(res_ptr)[index]);
+                    break;
+                case 4:
+                    ires = std::is_same<T, INT32>::value ?
+                           static_cast<int>(reinterpret_cast<INT32*>(res_ptr)[index]) :
+                           static_cast<int>(reinterpret_cast<UINT32*>(res_ptr)[index]);
+                    break;
+                default:
+                    _ASSERTE_MSG(false, "This should have exhausted all the possible sizes.");
+                    break;
+                }
+
                 OpStackSet<int>(arrInd, ires);
             }
             else
             {
-                OpStackSet<T>(arrInd, res);
+                OpStackSet<T>(arrInd, ((T*) res_ptr)[index]);
             }
         }
     }
@@ -9035,6 +9071,26 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T
             didIntrinsic = true;
         }
 
+// TODO: The following check for hardware intrinsics is not a production-level
+//       solution and may produce incorrect results.
+#ifdef FEATURE_INTERPRETER
+        static ConfigDWORD s_InterpreterHWIntrinsicsIsSupportedFalse;
+        if (s_InterpreterHWIntrinsicsIsSupportedFalse.val(CLRConfig::INTERNAL_InterpreterHWIntrinsicsIsSupportedFalse) != 0)
+        {
+            if (strcmp(methToCall->GetModule()->GetSimpleName(), "System.Private.CoreLib") == 0 &&
+#if defined(TARGET_X86) || defined(TARGET_AMD64)
+                strncmp(methToCall->GetClass()->GetDebugClassName(), "System.Runtime.Intrinsics.X86", 29) == 0 &&
+#elif defined(TARGET_ARM64)
+                strncmp(methToCall->GetClass()->GetDebugClassName(), "System.Runtime.Intrinsics.Arm", 29) == 0 &&
+#endif // defined(TARGET_X86) || defined(TARGET_AMD64)
+                strcmp(methToCall->GetName(), "get_IsSupported") == 0)
+            {
+                DoGetIsSupported();
+                didIntrinsic = true;
+            }
+        }
+#endif // FEATURE_INTERPRETER
+
 #if FEATURE_SIMD
         if (fFeatureSIMD.val(CLRConfig::EXTERNAL_FeatureSIMD) != 0)
         {
@@ -10615,6 +10671,20 @@ void Interpreter::DoSIMDHwAccelerated()
     LdIcon(1);
 }
 
+
+void Interpreter::DoGetIsSupported()
+{
+    CONTRACTL{
+        THROWS;
+        GC_TRIGGERS;
+        MODE_COOPERATIVE;
+    } CONTRACTL_END;
+
+    OpStackSet<BOOL>(m_curStackHt, false);
+    OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT));
+    m_curStackHt++;
+}
+
 void Interpreter::RecordConstrainedCall()
 {
     CONTRACTL {

diff --git a/src/coreclr/src/vm/interpreter.h b/src/coreclr/src/vm/interpreter.h
index fe9c7bc..424f36c 100644 (file)
--- a/src/coreclr/src/vm/interpreter.h
+++ b/src/coreclr/src/vm/interpreter.h
@@ -13,6 +13,8 @@
 #include "stack.h"
 #include "crst.h"
 #include "callhelpers.h"
+#include "codeversion.h"
+#include "clr_std/type_traits"
 
 typedef SSIZE_T NativeInt;
 typedef SIZE_T NativeUInt;
@@ -1767,6 +1769,7 @@ private:
     void DoByReferenceCtor();
     void DoByReferenceValue();
     void DoSIMDHwAccelerated();
+    void DoGetIsSupported();
 
     // Returns the proper generics context for use in resolving tokens ("precise" in the sense of including generic instantiation
     // information).

diff --git a/src/coreclr/src/vm/jitinterface.cpp b/src/coreclr/src/vm/jitinterface.cpp
index 9a4f119..e152f64 100644 (file)
--- a/src/coreclr/src/vm/jitinterface.cpp
+++ b/src/coreclr/src/vm/jitinterface.cpp
@@ -12248,15 +12248,19 @@ CorJitResult invokeCompileMethodHelper(EEJitManager *jitMgr,
 
 #ifdef FEATURE_INTERPRETER
     static ConfigDWORD s_InterpreterFallback;
+    static ConfigDWORD s_ForceInterpreter;
 
     bool isInterpreterStub   = false;
     bool interpreterFallback = (s_InterpreterFallback.val(CLRConfig::INTERNAL_InterpreterFallback) != 0);
+    bool forceInterpreter    = (s_ForceInterpreter.val(CLRConfig::INTERNAL_ForceInterpreter) != 0);
 
     if (interpreterFallback == false)
     {
         // If we're doing an "import_only" compilation, it's for verification, so don't interpret.
         // (We assume that importation is completely architecture-independent, or at least nearly so.)
-        if (FAILED(ret) && !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_IMPORT_ONLY) && !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MAKEFINALCODE))
+        if (FAILED(ret) &&
+            !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_IMPORT_ONLY) &&
+            (forceInterpreter || !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MAKEFINALCODE)))
         {
             if (SUCCEEDED(ret = Interpreter::GenerateInterpreterStub(comp, info, nativeEntry, nativeSizeOfCode)))
             {
@@ -12279,7 +12283,9 @@ CorJitResult invokeCompileMethodHelper(EEJitManager *jitMgr,
     {
         // If we're doing an "import_only" compilation, it's for verification, so don't interpret.
         // (We assume that importation is completely architecture-independent, or at least nearly so.)
-        if (FAILED(ret) && !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_IMPORT_ONLY) && !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MAKEFINALCODE))
+        if (FAILED(ret) &&
+            !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_IMPORT_ONLY) &&
+            (forceInterpreter || !jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MAKEFINALCODE)))
         {
             if (SUCCEEDED(ret = Interpreter::GenerateInterpreterStub(comp, info, nativeEntry, nativeSizeOfCode)))
             {