Enable constant CSE for ARM (dotnet#100054)

Also, extract out the code that determines if constant CSE or shared constant CSE are enabled, and rewrite it to be easier to understand.
Xinlong-Wu · Mar 22, 2024 · b549229 · b549229
1 parent 51782dc
commit b549229
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 50 deletions.
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -7083,6 +7083,9 @@ class Compiler
         return (enckey & ~TARGET_SIGN_BIT) << CSE_CONST_SHARED_LOW_BITS;
     }
 
+    static bool optSharedConstantCSEEnabled();
+    static bool optConstantCSEEnabled();
+
 /**************************************************************************
  *                   Value Number based CSEs
  *************************************************************************/

diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h
@@ -379,9 +379,9 @@ CONFIG_INTEGER(JitDisableSimdVN, W("JitDisableSimdVN"), 0) // Default 0, ValueNu
                                                            // If 3, disable both SIMD and HW Intrinsic nodes
 #endif                                                     // FEATURE_SIMD
 
-// Default 0, enable the CSE of Constants, including nearby offsets. (only for ARM64)
+// Default 0, enable the CSE of Constants, including nearby offsets. (only for ARM/ARM64)
 // If 1, disable all the CSE of Constants
-// If 2, enable the CSE of Constants but don't combine with nearby offsets. (only for ARM64)
+// If 2, enable the CSE of Constants but don't combine with nearby offsets. (only for ARM/ARM64)
 // If 3, enable the CSE of Constants including nearby offsets. (all platforms)
 // If 4, enable the CSE of Constants but don't combine with nearby offsets. (all platforms)
 //

diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp
@@ -499,26 +499,11 @@ unsigned optCSEKeyToHashIndex(size_t key, size_t optCSEhashSize)
 //
 unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt)
 {
-    size_t   key;
-    unsigned hval;
-    CSEdsc*  hashDsc;
-    bool     enableSharedConstCSE = false;
-    bool     isSharedConst        = false;
-    int      configValue          = JitConfig.JitConstCSE();
-
-#if defined(TARGET_ARMARCH)
-    // ARMARCH - allow to combine with nearby offsets, when config is not 2 or 4
-    if ((configValue != CONST_CSE_ENABLE_ARM_NO_SHARING) && (configValue != CONST_CSE_ENABLE_ALL_NO_SHARING))
-    {
-        enableSharedConstCSE = true;
-    }
-#endif // TARGET_ARMARCH
-
-    // All Platforms - also allow to combine with nearby offsets, when config is 3
-    if (configValue == CONST_CSE_ENABLE_ALL)
-    {
-        enableSharedConstCSE = true;
-    }
+    size_t     key;
+    unsigned   hval;
+    CSEdsc*    hashDsc;
+    const bool enableSharedConstCSE = optSharedConstantCSEEnabled();
+    bool       isSharedConst        = false;
 
     // We use the liberal Value numbers when building the set of CSE
     ValueNum vnLib     = tree->GetVN(VNK_Liberal);
@@ -1759,34 +1744,12 @@ void Compiler::optValnumCSE_Availability()
 //
 CSE_HeuristicCommon::CSE_HeuristicCommon(Compiler* pCompiler) : m_pCompiler(pCompiler)
 {
-    m_addCSEcount = 0; /* Count of the number of LclVars for CSEs that we added */
-    sortTab       = nullptr;
-    sortSiz       = 0;
-    madeChanges   = false;
-    codeOptKind   = m_pCompiler->compCodeOpt();
-
-    enableConstCSE = true;
-
-    int configValue = JitConfig.JitConstCSE();
-
-    // all platforms - disable CSE of constant values when config is 1
-    if (configValue == CONST_CSE_DISABLE_ALL)
-    {
-        enableConstCSE = false;
-    }
-
-#if !defined(TARGET_ARM64)
-    // non-ARM64 platforms - disable by default
-    //
-    enableConstCSE = false;
-
-    // Check for the two enable cases for all platforms
-    //
-    if ((configValue == CONST_CSE_ENABLE_ALL) || (configValue == CONST_CSE_ENABLE_ALL_NO_SHARING))
-    {
-        enableConstCSE = true;
-    }
-#endif
+    m_addCSEcount  = 0; /* Count of the number of LclVars for CSEs that we added */
+    sortTab        = nullptr;
+    sortSiz        = 0;
+    madeChanges    = false;
+    codeOptKind    = m_pCompiler->compCodeOpt();
+    enableConstCSE = Compiler::optConstantCSEEnabled();
 
 #ifdef DEBUG
     // Track the order of CSEs done (candidate number)
@@ -5460,6 +5423,56 @@ void Compiler::optCleanupCSEs()
     }
 }
 
+//---------------------------------------------------------------------------
+// optSharedConstantCSEEnabled: Returns `true` if shared constant CSE is enabled.
+//
+// Notes: see `optConstantCSEEnabled` for detecting if general constant CSE is enabled.
+//
+// static
+bool Compiler::optSharedConstantCSEEnabled()
+{
+    bool enableSharedConstCSE = false;
+    int  configValue          = JitConfig.JitConstCSE();
+
+    if (configValue == CONST_CSE_ENABLE_ALL)
+    {
+        enableSharedConstCSE = true;
+    }
+#if defined(TARGET_ARMARCH)
+    else if (configValue == CONST_CSE_ENABLE_ARM)
+    {
+        enableSharedConstCSE = true;
+    }
+#endif // TARGET_ARMARCH
+
+    return enableSharedConstCSE;
+}
+
+//---------------------------------------------------------------------------
+// optConstantCSEEnabled: Returns `true` if constant CSE is enabled.
+//
+// Notes: see `optSharedConstantCSEEnabled` for detecting if shared constant CSE is enabled.
+//
+// static
+bool Compiler::optConstantCSEEnabled()
+{
+    bool enableConstCSE = false;
+    int  configValue    = JitConfig.JitConstCSE();
+
+    if ((configValue == CONST_CSE_ENABLE_ALL) || (configValue == CONST_CSE_ENABLE_ALL_NO_SHARING))
+    {
+        enableConstCSE = true;
+    }
+#if defined(TARGET_ARMARCH)
+    else if ((configValue == CONST_CSE_ENABLE_ARM) || (configValue == CONST_CSE_ENABLE_ARM_NO_SHARING))
+    {
+        enableConstCSE = true;
+    }
+#endif
+
+    return enableConstCSE;
+}
+
 #ifdef DEBUG
 
 /*****************************************************************************