Partial re-revert of dotnet#104336. Only JIT fixes are included. (dotnet#105596)

* Partial re-revert of dotnet#104336. Only JIT fixes are included.

* fix for stress issues

* comments and some cleanup

* JIT format

* keep existing code for x86

Author: VSadov

src/coreclr/jit/codegenarmarch.cpp

@@ -605,10 +605,7 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg)
 {
     noway_assert(compiler->gsGlobalSecurityCookieAddr || compiler->gsGlobalSecurityCookieVal);
 
-    // Make sure that the return register is reported as live GC-ref so that any GC that kicks in while
-    // executing GS cookie check will not collect the object pointed to by REG_INTRET (R0).
-    if (!pushReg && (compiler->info.compRetNativeType == TYP_REF))
-        gcInfo.gcRegGCrefSetCur |= RBM_INTRET;
+    assert(GetEmitter()->emitGCDisabled());
 
     // We need two temporary registers, to load the GS cookie values and compare them. We can't use
     // any argument registers if 'pushReg' is true (meaning we have a JMP call). They should be

src/coreclr/jit/codegencommon.cpp

@@ -1516,8 +1516,22 @@ void CodeGen::genExitCode(BasicBlock* block)
     bool jmpEpilog = block->HasFlag(BBF_HAS_JMP);
     if (compiler->getNeedsGSSecurityCookie())
     {
+#ifndef JIT32_GCENCODER
+        // At this point the gc info that we track in codegen is often incorrect,
+        // as it could be missing return registers or arg registers (in a case of tail call).
+        // GS cookie check will emit a call and that will pass our GC info to emit and potentially mess things up.
+        // While we could infer returns/args and force them to be live and it seems to work in JIT32_GCENCODER case,
+        // it appears to be nontrivial in more general case.
+        // So, instead, we just claim that the whole thing is not GC-interruptible.
+        // Effectively this starts the epilog a few instructions earlier.
+        //
+        // CONSIDER: is that a good place to be that codegen loses track of returns/args at this point?
+        GetEmitter()->emitDisableGC();
+#endif
+
         genEmitGSCookieCheck(jmpEpilog);
 
+#ifdef JIT32_GCENCODER
         if (jmpEpilog)
         {
             // Dev10 642944 -
@@ -1540,6 +1554,7 @@ void CodeGen::genExitCode(BasicBlock* block)
             GetEmitter()->emitThisGCrefRegs = GetEmitter()->emitInitGCrefRegs = gcInfo.gcRegGCrefSetCur;
             GetEmitter()->emitThisByrefRegs = GetEmitter()->emitInitByrefRegs = gcInfo.gcRegByrefSetCur;
         }
+#endif
     }
 
     genReserveEpilog(block);
@@ -4728,43 +4743,13 @@ void CodeGen::genReserveProlog(BasicBlock* block)
 
 void CodeGen::genReserveEpilog(BasicBlock* block)
 {
-    regMaskTP gcrefRegsArg = gcInfo.gcRegGCrefSetCur;
-    regMaskTP byrefRegsArg = gcInfo.gcRegByrefSetCur;
-
-    /* The return value is special-cased: make sure it goes live for the epilog */
-
-    bool jmpEpilog = block->HasFlag(BBF_HAS_JMP);
-
-    if (IsFullPtrRegMapRequired() && !jmpEpilog)
-    {
-        if (varTypeIsGC(compiler->info.compRetNativeType))
-        {
-            noway_assert(genTypeStSz(compiler->info.compRetNativeType) == genTypeStSz(TYP_I_IMPL));
-
-            gcInfo.gcMarkRegPtrVal(REG_INTRET, compiler->info.compRetNativeType);
-
-            switch (compiler->info.compRetNativeType)
-            {
-                case TYP_REF:
-                    gcrefRegsArg |= RBM_INTRET;
-                    break;
-                case TYP_BYREF:
-                    byrefRegsArg |= RBM_INTRET;
-                    break;
-                default:
-                    break;
-            }
-
-            JITDUMP("Extending return value GC liveness to epilog\n");
-        }
-    }
-
     JITDUMP("Reserving epilog IG for block " FMT_BB "\n", block->bbNum);
 
     assert(block != nullptr);
-    const VARSET_TP& gcrefVarsArg(GetEmitter()->emitThisGCrefVars);
-    GetEmitter()->emitCreatePlaceholderIG(IGPT_EPILOG, block, gcrefVarsArg, gcrefRegsArg, byrefRegsArg,
-                                          block->IsLast());
+    // We pass empty GC info, because epilog is always an extend IG and will ignore what we pass.
+    // Besides, at this point the GC info that we track in CodeGen is often incorrect.
+    // See comments in genExitCode for more info.
+    GetEmitter()->emitCreatePlaceholderIG(IGPT_EPILOG, block, VarSetOps::MakeEmpty(compiler), 0, 0, block->IsLast());
 }
 
 /*****************************************************************************

src/coreclr/jit/codegenloongarch64.cpp

@@ -4610,12 +4610,7 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg)
 {
     noway_assert(compiler->gsGlobalSecurityCookieAddr || compiler->gsGlobalSecurityCookieVal);
 
-    // Make sure that the return register is reported as live GC-ref so that any GC that kicks in while
-    // executing GS cookie check will not collect the object pointed to by REG_INTRET (A0).
-    if (!pushReg && (compiler->info.compRetNativeType == TYP_REF))
-    {
-        gcInfo.gcRegGCrefSetCur |= RBM_INTRET;
-    }
+    assert(GetEmitter()->emitGCDisabled());
 
     // We need two temporary registers, to load the GS cookie values and compare them. We can't use
     // any argument registers if 'pushReg' is true (meaning we have a JMP call). They should be

src/coreclr/jit/codegenriscv64.cpp

@@ -4664,12 +4664,7 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg)
 {
     noway_assert(compiler->gsGlobalSecurityCookieAddr || compiler->gsGlobalSecurityCookieVal);
 
-    // Make sure that the return register is reported as live GC-ref so that any GC that kicks in while
-    // executing GS cookie check will not collect the object pointed to by REG_INTRET (A0).
-    if (!pushReg && (compiler->info.compRetNativeType == TYP_REF))
-    {
-        gcInfo.gcRegGCrefSetCur |= RBM_INTRET;
-    }
+    assert(GetEmitter()->emitGCDisabled());
 
     // We need two temporary registers, to load the GS cookie values and compare them. We can't use
     // any argument registers if 'pushReg' is true (meaning we have a JMP call). They should be

src/coreclr/jit/codegenxarch.cpp

@@ -96,14 +96,11 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg)
 {
     noway_assert(compiler->gsGlobalSecurityCookieAddr || compiler->gsGlobalSecurityCookieVal);
 
-    // Make sure that EAX is reported as live GC-ref so that any GC that kicks in while
-    // executing GS cookie check will not collect the object pointed to by EAX.
-    //
-    // For Amd64 System V, a two-register-returned struct could be returned in RAX and RDX
-    // In such case make sure that the correct GC-ness of RDX is reported as well, so
-    // a GC object pointed by RDX will not be collected.
+#ifdef JIT32_GCENCODER
     if (!pushReg)
     {
+        // Make sure that EAX is reported as live GC-ref so that any GC that kicks in while
+        // executing GS cookie check will not collect the object pointed to by EAX.
         if (compiler->compMethodReturnsRetBufAddr())
         {
             // This is for returning in an implicit RetBuf.
@@ -126,6 +123,9 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg)
             }
         }
     }
+#else
+    assert(GetEmitter()->emitGCDisabled());
+#endif
 
     regNumber regGSCheck;
     regMaskTP regMaskGSCheck = RBM_NONE;

src/coreclr/jit/emit.cpp

@@ -10427,9 +10427,9 @@ regMaskTP emitter::emitGetGCRegsKilledByNoGCCall(CorInfoHelpFunc helper)
 //    of the last instruction in the region makes GC safe again.
 //    In particular - once the IP is on the first instruction, but not executed it yet,
 //    it is still safe to do GC.
-//    The only special case is when NoGC region is used for prologs/epilogs.
-//    In such case the GC info could be incorrect until the prolog completes and epilogs
-//    may have unwindability restrictions, so the first instruction cannot have GC.
+//    The only special case is when NoGC region is used for prologs.
+//    In such case the GC info could be incorrect until the prolog completes, so the first
+//    instruction cannot have GC.
 
 void emitter::emitDisableGC()
 {
@@ -10459,6 +10459,11 @@ void emitter::emitDisableGC()
     }
 }
 
+bool emitter::emitGCDisabled()
+{
+    return emitNoGCIG == true;
+}
+
 //------------------------------------------------------------------------
 // emitEnableGC(): Removes a request that the following instruction groups are not GC-interruptible.
 //

src/coreclr/jit/emit.h

@@ -2745,6 +2745,7 @@ class emitter
 #if !defined(JIT32_GCENCODER)
     void emitDisableGC();
     void emitEnableGC();
+    bool emitGCDisabled();
 #endif // !defined(JIT32_GCENCODER)
 
 #if defined(TARGET_XARCH)

src/coreclr/jit/emitinl.h

@@ -594,8 +594,7 @@ bool emitter::emitGenNoGCLst(Callback& cb)
             emitter::instrDesc* id = emitFirstInstrDesc(ig->igData);
             assert(id != nullptr);
             assert(id->idCodeSize() > 0);
-            if (!cb(ig->igFuncIdx, ig->igOffs, ig->igSize, id->idCodeSize(),
-                    ig->igFlags & (IGF_FUNCLET_PROLOG | IGF_FUNCLET_EPILOG | IGF_EPILOG)))
+            if (!cb(ig->igFuncIdx, ig->igOffs, ig->igSize, id->idCodeSize(), ig->igFlags & (IGF_FUNCLET_PROLOG)))
             {
                 return false;
             }

src/coreclr/jit/gcencode.cpp

@@ -4027,8 +4027,7 @@ class InterruptibleRangeReporter
     // Report everything between the previous region and the current
     // region as interruptible.
 
-    bool operator()(
-        unsigned igFuncIdx, unsigned igOffs, unsigned igSize, unsigned firstInstrSize, bool isInPrologOrEpilog)
+    bool operator()(unsigned igFuncIdx, unsigned igOffs, unsigned igSize, unsigned firstInstrSize, bool isInProlog)
     {
         if (igOffs < m_uninterruptibleEnd)
         {
@@ -4042,9 +4041,9 @@ class InterruptibleRangeReporter
         if (igOffs > m_uninterruptibleEnd)
         {
             // Once the first instruction in IG executes, we cannot have GC.
-            // But it is ok to have GC while the IP is on the first instruction, unless we are in prolog/epilog.
+            // But it is ok to have GC while the IP is on the first instruction, unless we are in prolog.
             unsigned interruptibleEnd = igOffs;
-            if (!isInPrologOrEpilog)
+            if (!isInProlog)
             {
                 interruptibleEnd += firstInstrSize;
             }

src/coreclr/nativeaot/Runtime/thread.cpp

@@ -675,10 +675,16 @@ void Thread::HijackCallback(NATIVE_CONTEXT* pThreadContext, void* pThreadToHijac
     if (runtime->IsConservativeStackReportingEnabled() ||
         codeManager->IsSafePoint(pvAddress))
     {
+        // IsUnwindable is precise on arm64, but can give false negatives on other architectures.
+        // (when IP is on the first instruction of an epilog, we still can unwind,
+        // but we can tell if the instruction is the first only if we can navigate instructions backwards and check)
+        // The preciseness of IsUnwindable is tracked in https://github.com/dotnet/runtime/issues/101932
+#if defined(TARGET_ARM64)
         // we may not be able to unwind in some locations, such as epilogs.
         // such locations should not contain safe points.
         // when scanning conservatively we do not need to unwind
         ASSERT(codeManager->IsUnwindable(pvAddress) || runtime->IsConservativeStackReportingEnabled());
+#endif
 
         // if we are not given a thread to hijack
         // perform in-line wait on the current thread