From e17f25c565340e21a7b47202d0fa593dd1921771 Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Wed, 15 Jan 2020 13:06:59 -0800
Subject: [PATCH] Jitstress fixes for tailcall tests (#1771)

Make STRESS_GENERIC_VARN more compatible with methods that make explicit tail
calls. Don't add gc checks for explicit tail calls, and remove the code in
morph that blocks tail calls if gc checks are active.

Update the tailcall test to to disable STRESS_UNSAFE_BUFFER_CHECKS. This can
be reverted when #341 is merged.

Fixes #1752.
---
 src/coreclr/src/jit/compiler.h                |  2 +-
 src/coreclr/src/jit/importer.cpp              | 25 +++++++++++++------
 src/coreclr/src/jit/morph.cpp                 |  9 -------
 .../src/JIT/Directed/tailcall/tailcall.ilproj | 10 ++++++++
 4 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h
index b51dfe6c97e1f..23c692e1ffdd9 100644
--- a/src/coreclr/src/jit/compiler.h
+++ b/src/coreclr/src/jit/compiler.h
@@ -4021,7 +4021,7 @@ class Compiler
     }
     void impLoadArg(unsigned ilArgNum, IL_OFFSET offset);
     void impLoadLoc(unsigned ilLclNum, IL_OFFSET offset);
-    bool impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE& opcode);
+    bool impReturnInstruction(int prefixFlags, OPCODE& opcode);
 
 #ifdef _TARGET_ARM_
     void impMarkLclDstNotPromotable(unsigned tmpNum, GenTree* op, CORINFO_CLASS_HANDLE hClass);
diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp
index 065dab3ba8125..132cf2bf082e4 100644
--- a/src/coreclr/src/jit/importer.cpp
+++ b/src/coreclr/src/jit/importer.cpp
@@ -11508,7 +11508,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
             case CEE_RET:
                 prefixFlags &= ~PREFIX_TAILCALL; // ret without call before it
             RET:
-                if (!impReturnInstruction(block, prefixFlags, opcode))
+                if (!impReturnInstruction(prefixFlags, opcode))
                 {
                     return; // abort
                 }
@@ -16254,7 +16254,7 @@ GenTree* Compiler::impAssignSmallStructTypeToVar(GenTree* op, CORINFO_CLASS_HAND
 //    registers return values to suitable temps.
 //
 // Arguments:
-//     op -- call returning a struct in a registers
+//     op -- call returning a struct in registers
 //     hClass -- class handle for struct
 //
 // Returns:
@@ -16278,11 +16278,20 @@ GenTree* Compiler::impAssignMultiRegTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE
 }
 #endif // FEATURE_MULTIREG_RET
 
-// do import for a return
-// returns false if inlining was aborted
-// opcode can be ret or call in the case of a tail.call
-bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE& opcode)
+//------------------------------------------------------------------------
+// impReturnInstruction: import a return or an explicit tail call
+//
+// Arguments:
+//     prefixFlags -- active IL prefixes
+//     opcode -- [in, out] IL opcode
+//
+// Returns:
+//     True if import was successful (may fail for some inlinees)
+//
+bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
 {
+    const bool isTailCall = (prefixFlags & PREFIX_TAILCALL) != 0;
+
     if (tiVerificationNeeded)
     {
         verVerifyThisPtrInitialised();
@@ -16334,7 +16343,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
                       (varTypeIsStruct(op2) && varTypeIsStruct(info.compRetType)));
 
 #ifdef DEBUG
-            if (opts.compGcChecks && info.compRetType == TYP_REF)
+            if (!isTailCall && opts.compGcChecks && (info.compRetType == TYP_REF))
             {
                 // DDB 3483  : JIT Stress: early termination of GC ref's life time in exception code path
                 // VSW 440513: Incorrect gcinfo on the return value under COMPlus_JitGCChecks=1 for methods with
@@ -16741,7 +16750,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
     }
 
     // We must have imported a tailcall and jumped to RET
-    if (prefixFlags & PREFIX_TAILCALL)
+    if (isTailCall)
     {
 #if defined(FEATURE_CORECLR) || !defined(_TARGET_AMD64_)
         // Jit64 compat:
diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp
index c8042fc1acaf7..12b08920685d9 100644
--- a/src/coreclr/src/jit/morph.cpp
+++ b/src/coreclr/src/jit/morph.cpp
@@ -6975,15 +6975,6 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
     }
 #endif
 
-#ifdef DEBUG
-    // DDB 99324: Just disable tailcall under compGcChecks stress mode.
-    if (opts.compGcChecks)
-    {
-        failTailCall("GcChecks");
-        return nullptr;
-    }
-#endif
-
     // We have to ensure to pass the incoming retValBuf as the
     // outgoing one. Using a temp will not do as this function will
     // not regain control to do the copy. This can happen when inlining
diff --git a/src/coreclr/tests/src/JIT/Directed/tailcall/tailcall.ilproj b/src/coreclr/tests/src/JIT/Directed/tailcall/tailcall.ilproj
index bcaa593ba8113..f21adf8eb35a9 100644
--- a/src/coreclr/tests/src/JIT/Directed/tailcall/tailcall.ilproj
+++ b/src/coreclr/tests/src/JIT/Directed/tailcall/tailcall.ilproj
@@ -9,4 +9,14 @@
   <ItemGroup>
     <Compile Include="$(MSBuildProjectName).il" />
   </ItemGroup>
+  <PropertyGroup>
+    <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set COMPlus_JitStressModeNamesNot=STRESS_UNSAFE_BUFFER_CHECKS
+]]></CLRTestBatchPreCommands>
+    <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export COMPlus_JitStressModeNamesNot=STRESS_UNSAFE_BUFFER_CHECKS
+]]></BashCLRTestPreCommands>
+  </PropertyGroup>
 </Project>