From 046f84c990a04e22227c371863ef52796e437a56 Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Fri, 3 Jun 2022 12:53:52 -0700
Subject: [PATCH] JIT: broaden cloning invariant checks

Streamline call effects checks. Use wider bit vectors.

Closes #70100.
---
 src/coreclr/jit/optimizer.cpp                 | 70 +++++--------------
 src/coreclr/jit/varset.h                      |  5 +-
 src/tests/JIT/opt/Cloning/callandindir.cs     | 54 ++++++++++++++
 src/tests/JIT/opt/Cloning/callandindir.csproj |  9 +++
 4 files changed, 84 insertions(+), 54 deletions(-)
 create mode 100644 src/tests/JIT/opt/Cloning/callandindir.cs
 create mode 100644 src/tests/JIT/opt/Cloning/callandindir.csproj

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 8809c499d4c3a..ca034a141dbb3 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -5958,60 +5958,26 @@ bool Compiler::optIsSetAssgLoop(unsigned lnum, ALLVARSET_VALARG_TP vars, varRefK
         return true;
     }
 
-    switch (loop->lpAsgCall)
+    // If caller is worried about possible indirect effects, check
+    // what we know about the calls in the loop.
+    //
+    if (inds != 0)
     {
-        case CALLINT_ALL:
-
-            /* Can't hoist if the call might have side effect on an indirection. */
-
-            if (loop->lpAsgInds != VR_NONE)
-            {
-                return true;
-            }
-
-            break;
-
-        case CALLINT_REF_INDIRS:
-
-            /* Can't hoist if the call might have side effect on an ref indirection. */
-
-            if (loop->lpAsgInds & VR_IND_REF)
-            {
-                return true;
-            }
-
-            break;
-
-        case CALLINT_SCL_INDIRS:
-
-            /* Can't hoist if the call might have side effect on an non-ref indirection. */
-
-            if (loop->lpAsgInds & VR_IND_SCL)
-            {
-                return true;
-            }
-
-            break;
-
-        case CALLINT_ALL_INDIRS:
-
-            /* Can't hoist if the call might have side effect on any indirection. */
-
-            if (loop->lpAsgInds & (VR_IND_REF | VR_IND_SCL))
-            {
+        switch (loop->lpAsgCall)
+        {
+            case CALLINT_ALL:
                 return true;
-            }
-
-            break;
-
-        case CALLINT_NONE:
-
-            /* Other helpers kill nothing */
-
-            break;
-
-        default:
-            noway_assert(!"Unexpected lpAsgCall value");
+            case CALLINT_REF_INDIRS:
+                return (inds & VR_IND_REF) != 0;
+            case CALLINT_SCL_INDIRS:
+                return (inds & VR_IND_SCL) != 0;
+            case CALLINT_ALL_INDIRS:
+                return (inds & (VR_IND_REF | VR_IND_SCL)) != 0;
+            case CALLINT_NONE:
+                return false;
+            default:
+                noway_assert(!"Unexpected lpAsgCall value");
+        }
     }
 
     return false;
diff --git a/src/coreclr/jit/varset.h b/src/coreclr/jit/varset.h
index 88f8639437667..c4cba1b57797a 100644
--- a/src/coreclr/jit/varset.h
+++ b/src/coreclr/jit/varset.h
@@ -111,7 +111,7 @@ typedef BitSetOpsWithCounter<VARSET_TP,
 typedef VarSetOpsRaw       VarSetOps;
 #endif
 
-#define ALLVARSET_REP BSUInt64
+#define ALLVARSET_REP BSShortLong
 
 #if ALLVARSET_REP == BSUInt64
 
@@ -141,7 +141,8 @@ typedef BitSetOps</*BitSetType*/ BitSetShortLongRep,
 
 typedef BitSetShortLongRep ALLVARSET_TP;
 
-const unsigned lclMAX_ALLSET_TRACKED = lclMAX_TRACKED;
+// default value for JitConfig.JitMaxLocalsToTrack()
+const unsigned lclMAX_ALLSET_TRACKED = 0x400;
 
 #define ALLVARSET_REP_IS_CLASS 0
 
diff --git a/src/tests/JIT/opt/Cloning/callandindir.cs b/src/tests/JIT/opt/Cloning/callandindir.cs
new file mode 100644
index 0000000000000..065c893e11530
--- /dev/null
+++ b/src/tests/JIT/opt/Cloning/callandindir.cs
@@ -0,0 +1,54 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+
+// Loops in F, G, H should all clone
+
+class CallAndIndir
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void S() { }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void F(int[] a, int low, int high, ref int z)
+    {
+        for (int i = low; i < high; i++)
+        {
+             z += a[i];
+             S(); 
+        }  
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void G(int[] a, int low, int high, ref int z)
+    {
+        for (int i = low; i < high; i++)
+        {
+             z += a[i];
+        }  
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void H(int[] a, int low, int high, ref int z)
+    {
+        int r = 0;
+        for (int i = low; i < high; i++)
+        {
+             r += a[i];
+             S();
+        }  
+        z += r;
+    }
+
+    public static int Main()
+    {
+         int[] a = new int[] { 1, 2, 3, 4 };
+         int z = 0;
+         F(a, 2, 4, ref z);
+         G(a, 2, 4, ref z);
+         H(a, 2, 4, ref z);
+         return z + 79;
+    }
+}
diff --git a/src/tests/JIT/opt/Cloning/callandindir.csproj b/src/tests/JIT/opt/Cloning/callandindir.csproj
new file mode 100644
index 0000000000000..6946bed81bfd5
--- /dev/null
+++ b/src/tests/JIT/opt/Cloning/callandindir.csproj
@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>