Eliminate duplicate zero initializations more aggressively. (#31960)

We have 3 places in the code where we may need to insert explicit zero
initialization. In some cases the initialization may become redundant with prolog
initialization so we have logic to avoid explicit initialization in some cases.
This change makes the logic less conservative.

1. If the variable is not being initialized in a loop, we can avoid explicit zero initialization if
      - the variable is a gc pointer, or
      - the variable is a struct with gc pointer fields and either all fields are gc pointer fields
          or the struct is big enough to guarantee block initialization, or
      - compInitMem is set and the variable has a long lifetime or has gc fields.

   Before this change we always inserted explicit zero initalization when compInitMem
   wasn't set and the variable was a gc pointer or a struct with gc fields. This relaxes that
   as described above.

2. When we were allocating structs via newobj, we were always inserting explicit zero initializations
   when importing an inlinee. This change applies the same optimization if the basic block can't be
   in a loop after inlining.

3. When we were initializing inlinee locals, we were only optimizing explicit zero initializations
   for structs; however, the same logic applies to non-structs.

4. If the initialization basic block is in a loop but is BBJ_RETURN, we may avoid zero initialization
   since the block will be executed at most once per method invocation.

diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp
index fe2b5de..f152593 100644 (file)
--- a/src/coreclr/src/jit/codegencommon.cpp
+++ b/src/coreclr/src/jit/codegencommon.cpp
@@ -4666,6 +4666,9 @@ void CodeGen::genCheckUseBlockInit()
             continue;
         }
 
+// TODO-Review: The code below is currently unreachable. We are guaranteed to execute one of the
+// 'continue' statements above.
+#if 0
         /* If we don't know lifetimes of variables, must be conservative */
         if (!compiler->backendRequiresLocalVarLifetimes())
         {
@@ -4700,6 +4703,7 @@ void CodeGen::genCheckUseBlockInit()
         {
             largeGcStructs++;
         }
+#endif
     }
 
     /* Don't forget about spill temps that hold pointers */
@@ -4728,6 +4732,11 @@ void CodeGen::genCheckUseBlockInit()
     // Secondary factor is the presence of large structs that
     // potentially only need some fields set to zero. We likely don't
     // model this very well, but have left the logic as is for now.
+
+    // Compiler::fgVarNeedsExplicitZeroInit relies on this logic to
+    // find structs that are guaranteed to be block initialized.
+    // If this logic changes, Compiler::fgVarNeedsExplicitZeroInit needs
+    // to be modified.
     CLANG_FORMAT_COMMENT_ANCHOR;
 
 #ifdef TARGET_64BIT

diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h
index c974232..112ceea 100644 (file)
--- a/src/coreclr/src/jit/compiler.h
+++ b/src/coreclr/src/jit/compiler.h
@@ -4542,8 +4542,8 @@ public:
 
     unsigned fgSsaPassesCompleted; // Number of times fgSsaBuild has been run.
 
-    // Returns "true" if a struct temp of the given type requires needs zero init in this block
-    inline bool fgStructTempNeedsExplicitZeroInit(LclVarDsc* varDsc, BasicBlock* block);
+    // Returns "true" if the variable needs explicit zero initialization.
+    inline bool fgVarNeedsExplicitZeroInit(LclVarDsc* varDsc, bool bbInALoop, bool bbIsReturn);
 
     // The value numbers for this compilation.
     ValueNumStore* vnStore;

diff --git a/src/coreclr/src/jit/compiler.hpp b/src/coreclr/src/jit/compiler.hpp
index d0de0f1..889d501 100644 (file)
--- a/src/coreclr/src/jit/compiler.hpp
+++ b/src/coreclr/src/jit/compiler.hpp
@@ -4106,26 +4106,59 @@ inline void Compiler::CLR_API_Leave(API_ICorJitInfo_Names ename)
 #endif // MEASURE_CLRAPI_CALLS
 
 //------------------------------------------------------------------------------
-// fgStructTempNeedsExplicitZeroInit : Check whether temp struct needs
-//                                     explicit zero initialization in this basic block.
+// fgVarNeedsExplicitZeroInit : Check whether the variable needs an explicit zero initialization.
 //
 // Arguments:
-//    varDsc -           struct local var description
-//    block  -           basic block to check
+//    varDsc     -       local var description
+//    bbInALoop  -       true if the basic block may be in a loop
+//    bbIsReturn -       true if the basic block always returns
 //
 // Returns:
-//             true if the struct temp needs explicit zero-initialization in this basic block;
+//             true if the var needs explicit zero-initialization in this basic block;
 //             false otherwise
 //
 // Notes:
-//     If compInitMem is true, structs with GC pointer fields and long-lifetime structs
-//     are fully zero-initialized in the prologue. Therefore, we don't need to insert
-//     zero-initialization in this block if it is not in a loop.
+//     If the variable is not being initialized in a loop, we can avoid explicit zero initialization if
+//      - the variable is a gc pointer, or
+//      - the variable is a struct with gc pointer fields and either all fields are gc pointer fields
+//           or the struct is big enough to guarantee block initialization, or
+//      - compInitMem is set and the variable has a long lifetime or has gc fields.
+//     In these cases we will insert zero-initialization in the prolog if necessary.
 
-bool Compiler::fgStructTempNeedsExplicitZeroInit(LclVarDsc* varDsc, BasicBlock* block)
+bool Compiler::fgVarNeedsExplicitZeroInit(LclVarDsc* varDsc, bool bbInALoop, bool bbIsReturn)
 {
-    return !info.compInitMem || ((block->bbFlags & BBF_BACKWARD_JUMP) != 0) ||
-           (!varDsc->HasGCPtr() && varDsc->lvIsTemp);
+    if (bbInALoop && !bbIsReturn)
+    {
+        return true;
+    }
+
+    if (varTypeIsGC(varDsc->lvType))
+    {
+        return false;
+    }
+
+    if ((varDsc->lvType == TYP_STRUCT) && varDsc->HasGCPtr())
+    {
+        ClassLayout* layout = varDsc->GetLayout();
+        if (layout->GetSlotCount() == layout->GetGCPtrCount())
+        {
+            return false;
+        }
+
+// Below conditions guarantee block initialization, which will initialize
+// all struct fields. If the logic for block initialization in CodeGen::genCheckUseBlockInit()
+// changes, these conditions need to be updated.
+#ifdef TARGET_64BIT
+        if (roundUp(varDsc->lvSize(), TARGET_POINTER_SIZE) / sizeof(int) > 8)
+#else
+        if (roundUp(varDsc->lvSize(), TARGET_POINTER_SIZE) / sizeof(int) > 4)
+#endif
+        {
+            return false;
+        }
+    }
+
+    return !info.compInitMem || (varDsc->lvIsTemp && !varDsc->HasGCPtr());
 }
 
 /*****************************************************************************/

diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp
index 23bc843..6cbab31 100644 (file)
--- a/src/coreclr/src/jit/flowgraph.cpp
+++ b/src/coreclr/src/jit/flowgraph.cpp
@@ -23581,10 +23581,15 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
 
     CORINFO_METHOD_INFO* InlineeMethodInfo = InlineeCompiler->info.compMethodInfo;
 
-    unsigned lclCnt = InlineeMethodInfo->locals.numArgs;
+    unsigned lclCnt     = InlineeMethodInfo->locals.numArgs;
+    bool     bbInALoop  = (block->bbFlags & BBF_BACKWARD_JUMP) != 0;
+    bool     bbIsReturn = block->bbJumpKind == BBJ_RETURN;
 
-    // Does callee contain any zero-init local?
-    if ((lclCnt != 0) && (InlineeMethodInfo->options & CORINFO_OPT_INIT_LOCALS) != 0)
+    // If the callee contains zero-init locals, we need to explicitly initialize them if we are
+    // in a loop or if the caller doesn't have compInitMem set. Otherwise we can rely on the
+    // normal logic in the caller to insert zero-init in the prolog if necessary.
+    if ((lclCnt != 0) && ((InlineeMethodInfo->options & CORINFO_OPT_INIT_LOCALS) != 0) &&
+        ((bbInALoop && !bbIsReturn) || !info.compInitMem))
     {
 
 #ifdef DEBUG
@@ -23598,9 +23603,20 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
         {
             unsigned tmpNum = inlineInfo->lclTmpNum[lclNum];
 
-            // Is the local used at all?
+            // If the local is used check whether we need to insert explicit zero initialization.
             if (tmpNum != BAD_VAR_NUM)
             {
+                if (!fgVarNeedsExplicitZeroInit(lvaGetDesc(tmpNum), bbInALoop, bbIsReturn))
+                {
+#ifdef DEBUG
+                    if (verbose)
+                    {
+                        printf("\nSkipping zero initialization of V%02u\n", tmpNum);
+                    }
+#endif // DEBUG
+                    continue;
+                }
+
                 var_types lclTyp = (var_types)lvaTable[tmpNum].lvType;
                 noway_assert(lclTyp == lclVarInfo[lclNum + inlineInfo->argCnt].lclTypeInfo);
 
@@ -23615,18 +23631,14 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
                 {
                     CORINFO_CLASS_HANDLE structType =
                         lclVarInfo[lclNum + inlineInfo->argCnt].lclVerTypeInfo.GetClassHandle();
-
-                    if (fgStructTempNeedsExplicitZeroInit(lvaTable + tmpNum, block))
-                    {
-                        tree = gtNewBlkOpNode(gtNewLclvNode(tmpNum, lclTyp), // Dest
-                                              gtNewIconNode(0),              // Value
-                                              false,                         // isVolatile
-                                              false);                        // not copyBlock
-
-                        newStmt = gtNewStmt(tree, callILOffset);
-                        fgInsertStmtAfter(block, afterStmt, newStmt);
-                        afterStmt = newStmt;
-                    }
+                    tree = gtNewBlkOpNode(gtNewLclvNode(tmpNum, lclTyp), // Dest
+                                          gtNewIconNode(0),              // Value
+                                          false,                         // isVolatile
+                                          false);                        // not copyBlock
+
+                    newStmt = gtNewStmt(tree, callILOffset);
+                    fgInsertStmtAfter(block, afterStmt, newStmt);
+                    afterStmt = newStmt;
                 }
 
 #ifdef DEBUG

diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp
index d828dda..d2a5e12 100644 (file)
--- a/src/coreclr/src/jit/importer.cpp
+++ b/src/coreclr/src/jit/importer.cpp
@@ -13821,7 +13821,13 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                             // and potentially exploitable.
                             lvaSetStruct(lclNum, resolvedToken.hClass, true /* unsafe value cls check */);
                         }
-                        if (compIsForInlining() || fgStructTempNeedsExplicitZeroInit(lvaTable + lclNum, block))
+
+                        bool bbInALoop =
+                            (compIsForInlining() && ((impInlineInfo->iciBlock->bbFlags & BBF_BACKWARD_JUMP) != 0)) ||
+                            ((block->bbFlags & BBF_BACKWARD_JUMP) != 0);
+                        bool bbIsReturn = (block->bbJumpKind == BBJ_RETURN) &&
+                                          (!compIsForInlining() || (impInlineInfo->iciBlock->bbJumpKind == BBJ_RETURN));
+                        if (fgVarNeedsExplicitZeroInit(lvaGetDesc(lclNum), bbInALoop, bbIsReturn))
                         {
                             // Append a tree to zero-out the temp
                             newObjThisPtr = gtNewLclvNode(lclNum, lvaTable[lclNum].TypeGet());

diff --git a/src/coreclr/src/jit/objectalloc.cpp b/src/coreclr/src/jit/objectalloc.cpp
index e5cccf2..79a2685 100644 (file)
--- a/src/coreclr/src/jit/objectalloc.cpp
+++ b/src/coreclr/src/jit/objectalloc.cpp
@@ -510,8 +510,10 @@ unsigned int ObjectAllocator::MorphAllocObjNodeIntoStackAlloc(GenTreeAllocObj* a
     const int unsafeValueClsCheck = true;
     comp->lvaSetStruct(lclNum, allocObj->gtAllocObjClsHnd, unsafeValueClsCheck);
 
-    // Initialize the object memory if necessary
-    if (comp->fgStructTempNeedsExplicitZeroInit(comp->lvaTable + lclNum, block))
+    // Initialize the object memory if necessary.
+    bool bbInALoop  = (block->bbFlags & BBF_BACKWARD_JUMP) != 0;
+    bool bbIsReturn = block->bbJumpKind == BBJ_RETURN;
+    if (comp->fgVarNeedsExplicitZeroInit(comp->lvaGetDesc(lclNum), bbInALoop, bbIsReturn))
     {
         //------------------------------------------------------------------------
         // STMTx (IL 0x... ???)