JIT: Fix tailcall-to-loop improper locals zeroing (#81083)
authorJakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
Wed, 25 Jan 2023 08:05:29 +0000 (09:05 +0100)
committerGitHub <noreply@github.com>
Wed, 25 Jan 2023 08:05:29 +0000 (09:05 +0100)
The zeroing that the tailcall-to-loop optimization does was zeroing the
promoted copies for implicit byrefs even when promotion of them was undone.
This was introducing unexpected references to the promoted fields.

Fix #81081

src/coreclr/jit/morph.cpp
src/tests/JIT/Regression/JitBlue/Runtime_81081/Runtime_81081.cs [new file with mode: 0644]
src/tests/JIT/Regression/JitBlue/Runtime_81081/Runtime_81081.csproj [new file with mode: 0644]

index 21d87671a41438560f7af94a97a9f63df32b39dc..21d2117447da79d76798b43a1b2dc2ba163d42c5 100644 (file)
@@ -7866,9 +7866,7 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa
     // Liveness phase will remove unnecessary initializations.
     if (info.compInitMem || compSuppressedZeroInit)
     {
-        unsigned   varNum;
-        LclVarDsc* varDsc;
-        for (varNum = 0, varDsc = lvaTable; varNum < lvaCount; varNum++, varDsc++)
+        for (unsigned varNum = 0; varNum < lvaCount; varNum++)
         {
 #if FEATURE_FIXED_OUT_ARGS
             if (varNum == lvaOutgoingArgSpaceVar)
@@ -7876,29 +7874,55 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa
                 continue;
             }
 #endif // FEATURE_FIXED_OUT_ARGS
-            if (!varDsc->lvIsParam)
+
+            LclVarDsc* varDsc = lvaGetDesc(varNum);
+
+            if (varDsc->lvIsParam)
             {
-                var_types lclType            = varDsc->TypeGet();
-                bool      isUserLocal        = (varNum < info.compLocalsCount);
-                bool      structWithGCFields = ((lclType == TYP_STRUCT) && varDsc->GetLayout()->HasGCPtr());
-                bool      hadSuppressedInit  = varDsc->lvSuppressedZeroInit;
-                if ((info.compInitMem && (isUserLocal || structWithGCFields)) || hadSuppressedInit)
+                continue;
+            }
+
+#if FEATURE_IMPLICIT_BYREFS
+            if (varDsc->lvPromoted)
+            {
+                LclVarDsc* firstField = lvaGetDesc(varDsc->lvFieldLclStart);
+                if (firstField->lvParentLcl != varNum)
                 {
-                    GenTree* lcl  = gtNewLclvNode(varNum, lclType);
-                    GenTree* init = nullptr;
-                    if (varTypeIsStruct(lclType))
-                    {
-                        init = gtNewBlkOpNode(lcl, gtNewIconNode(0));
-                        init = fgMorphInitBlock(init);
-                    }
-                    else
-                    {
-                        GenTree* zero = gtNewZeroConNode(lclType);
-                        init          = gtNewAssignNode(lcl, zero);
-                    }
-                    Statement* initStmt = gtNewStmt(init, callDI);
-                    fgInsertStmtBefore(block, lastStmt, initStmt);
+                    // Local copy for implicit byref promotion that was undone. Do
+                    // not introduce new references to it, all uses have been
+                    // morphed to access the parameter.
+                    CLANG_FORMAT_COMMENT_ANCHOR;
+
+#ifdef DEBUG
+                    LclVarDsc* param = lvaGetDesc(firstField->lvParentLcl);
+                    assert(param->lvIsImplicitByRef && !param->lvPromoted);
+                    assert(param->lvFieldLclStart == varNum);
+#endif
+                    continue;
+                }
+            }
+#endif
+
+            var_types lclType            = varDsc->TypeGet();
+            bool      isUserLocal        = (varNum < info.compLocalsCount);
+            bool      structWithGCFields = ((lclType == TYP_STRUCT) && varDsc->GetLayout()->HasGCPtr());
+            bool      hadSuppressedInit  = varDsc->lvSuppressedZeroInit;
+            if ((info.compInitMem && (isUserLocal || structWithGCFields)) || hadSuppressedInit)
+            {
+                GenTree* lcl  = gtNewLclvNode(varNum, lclType);
+                GenTree* init = nullptr;
+                if (varTypeIsStruct(lclType))
+                {
+                    init = gtNewBlkOpNode(lcl, gtNewIconNode(0));
+                    init = fgMorphInitBlock(init);
+                }
+                else
+                {
+                    GenTree* zero = gtNewZeroConNode(lclType);
+                    init          = gtNewAssignNode(lcl, zero);
                 }
+                Statement* initStmt = gtNewStmt(init, callDI);
+                fgInsertStmtBefore(block, lastStmt, initStmt);
             }
         }
     }
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_81081/Runtime_81081.cs b/src/tests/JIT/Regression/JitBlue/Runtime_81081/Runtime_81081.cs
new file mode 100644 (file)
index 0000000..3ca57de
--- /dev/null
@@ -0,0 +1,25 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+class Runtime_81081
+{
+    static int Main()
+    {
+        Test(1234, default);
+        return 100;
+    }
+
+    static int Test(int count, S16 s)
+    {
+        object o = "1234";
+        if (count == 0 || o.GetHashCode() == 1234)
+            return 42;
+
+        return Test(count - 1, s);
+    }
+
+    struct S16
+    {
+        public object A, B;
+    }
+}
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_81081/Runtime_81081.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_81081/Runtime_81081.csproj
new file mode 100644 (file)
index 0000000..35f4225
--- /dev/null
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <CLRTestEnvironmentVariable Include="DOTNET_JitNoInline" Value="1" />
+  </ItemGroup>
+</Project>