From c77ad480298300b655ced317bf25b1fe4f27ceb3 Mon Sep 17 00:00:00 2001 From: Eugene Rozenfeld Date: Tue, 12 Sep 2017 17:15:08 -0700 Subject: [PATCH] Remove redundant zero-initialization of struct temps with GC fields. (#13868) Remove redundant zero-initialization of struct temps with GC fields. Structs with GC pointer fields are fully zero-initialized in the prolog if compInitMem is true. Therefore, we don't need to insert zero-initialization for the result of newobj or for inlinee locals when they are structs with GC pointer fields and the basic bock is not in a loop. --- src/jit/compiler.h | 3 + src/jit/compiler.hpp | 22 +++++ src/jit/flowgraph.cpp | 19 ++-- src/jit/importer.cpp | 24 ++--- src/jit/morph.cpp | 40 ++++---- .../JitBlue/GitHub_13822/GitHub_13822.il | 102 +++++++++++++++++++++ .../JitBlue/GitHub_13822/GitHub_13822.ilproj | 35 +++++++ 7 files changed, 208 insertions(+), 37 deletions(-) create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.il create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.ilproj diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 0bf6950..b4077cf 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -3938,6 +3938,9 @@ public: // Returns "true" iff lcl "lclNum" should be excluded from SSA. inline bool fgExcludeFromSsa(unsigned lclNum); + // Returns "true" if a struct temp of the given type requires needs zero init in this block + inline bool fgStructTempNeedsExplicitZeroInit(LclVarDsc* varDsc, BasicBlock* block); + // The value numbers for this compilation. ValueNumStore* vnStore; diff --git a/src/jit/compiler.hpp b/src/jit/compiler.hpp index b838933..4a90760 100644 --- a/src/jit/compiler.hpp +++ b/src/jit/compiler.hpp @@ -4711,6 +4711,28 @@ 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. +// +// Arguments: +// varDsc - struct local var description +// block - basic block to check +// +// Returns: +// true if the struct temp needs explicit zero-initialization in this basic block; +// false otherwise +// +// Notes: +// Structs with GC pointer fields are fully zero-initialized in the prolog if compInitMem is true. +// Therefore, we don't need to insert zero-initialization if this block is not in a loop. + +bool Compiler::fgStructTempNeedsExplicitZeroInit(LclVarDsc* varDsc, BasicBlock* block) +{ + bool containsGCPtr = (varDsc->lvStructGcCount > 0); + return (!containsGCPtr || !info.compInitMem || ((block->bbFlags & BBF_BACKWARD_JUMP) != 0)); +} + /*****************************************************************************/ bool Compiler::fgExcludeFromSsa(unsigned lclNum) { diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index 9d69595..8b6cb81 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -23107,14 +23107,17 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) CORINFO_CLASS_HANDLE structType = lclVarInfo[lclNum + inlineInfo->argCnt].lclVerTypeInfo.GetClassHandle(); - tree = gtNewBlkOpNode(gtNewLclvNode(tmpNum, lclTyp), // Dest - gtNewIconNode(0), // Value - info.compCompHnd->getClassSize(structType), // Size - false, // isVolatile - false); // not copyBlock - - newStmt = gtNewStmt(tree, callILOffset); - afterStmt = fgInsertStmtAfter(block, afterStmt, newStmt); + if (fgStructTempNeedsExplicitZeroInit(lvaTable + tmpNum, block)) + { + tree = gtNewBlkOpNode(gtNewLclvNode(tmpNum, lclTyp), // Dest + gtNewIconNode(0), // Value + info.compCompHnd->getClassSize(structType), // Size + false, // isVolatile + false); // not copyBlock + + newStmt = gtNewStmt(tree, callILOffset); + afterStmt = fgInsertStmtAfter(block, afterStmt, newStmt); + } } #ifdef DEBUG diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 8e7ecde..a8ca950 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -6989,7 +6989,7 @@ var_types Compiler::impImportCall(OPCODE opcode, exactContextHnd = callInfo->contextHandle; exactContextNeedsRuntimeLookup = callInfo->exactContextNeedsRuntimeLookup == TRUE; - // Recursive call is treaded as a loop to the begining of the method. + // Recursive call is treated as a loop to the begining of the method. if (methHnd == info.compMethodHnd) { #ifdef DEBUG @@ -12994,16 +12994,18 @@ void Compiler::impImportBlockCode(BasicBlock* block) // and potentially exploitable. lvaSetStruct(lclNum, resolvedToken.hClass, true /* unsafe value cls check */); } - - // Append a tree to zero-out the temp - newObjThisPtr = gtNewLclvNode(lclNum, lvaTable[lclNum].TypeGet()); - - newObjThisPtr = gtNewBlkOpNode(newObjThisPtr, // Dest - gtNewIconNode(0), // Value - size, // Size - false, // isVolatile - false); // not copyBlock - impAppendTree(newObjThisPtr, (unsigned)CHECK_SPILL_NONE, impCurStmtOffs); + if (compIsForInlining() || fgStructTempNeedsExplicitZeroInit(lvaTable + lclNum, block)) + { + // Append a tree to zero-out the temp + newObjThisPtr = gtNewLclvNode(lclNum, lvaTable[lclNum].TypeGet()); + + newObjThisPtr = gtNewBlkOpNode(newObjThisPtr, // Dest + gtNewIconNode(0), // Value + size, // Size + false, // isVolatile + false); // not copyBlock + impAppendTree(newObjThisPtr, (unsigned)CHECK_SPILL_NONE, impCurStmtOffs); + } // Obtain the address of the temp newObjThisPtr = diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 591bebb..8225108 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -8075,33 +8075,37 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa // If compInitMem is set, we may need to zero-initialize some locals. Normally it's done in the prolog // but this loop can't include the prolog. Since we don't have liveness information, we insert zero-initialization - // for all non-parameter non-temp locals. Liveness phase will remove unnecessary initializations. + // for all non-parameter non-temp locals as well as temp structs with GC fields. + // Liveness phase will remove unnecessary initializations. if (info.compInitMem) { unsigned varNum; LclVarDsc* varDsc; - for (varNum = 0, varDsc = lvaTable; varNum < info.compLocalsCount; varNum++, varDsc++) + for (varNum = 0, varDsc = lvaTable; varNum < lvaCount; varNum++, varDsc++) { + var_types lclType = varDsc->TypeGet(); if (!varDsc->lvIsParam) { - assert(!varDsc->lvIsTemp); - var_types lclType = varDsc->TypeGet(); - GenTreePtr lcl = gtNewLclvNode(varNum, lclType); - GenTreePtr init = nullptr; - if (lclType == TYP_STRUCT) + if (!varDsc->lvIsTemp || ((lclType == TYP_STRUCT) && (varDsc->lvStructGcCount > 0))) { - const bool isVolatile = false; - const bool isCopyBlock = false; - init = gtNewBlkOpNode(lcl, gtNewIconNode(0), varDsc->lvSize(), isVolatile, isCopyBlock); - init = fgMorphInitBlock(init); - } - else - { - GenTreePtr zero = gtNewZeroConNode(genActualType(lclType)); - init = gtNewAssignNode(lcl, zero); + var_types lclType = varDsc->TypeGet(); + GenTreePtr lcl = gtNewLclvNode(varNum, lclType); + GenTreePtr init = nullptr; + if (lclType == TYP_STRUCT) + { + const bool isVolatile = false; + const bool isCopyBlock = false; + init = gtNewBlkOpNode(lcl, gtNewIconNode(0), varDsc->lvSize(), isVolatile, isCopyBlock); + init = fgMorphInitBlock(init); + } + else + { + GenTreePtr zero = gtNewZeroConNode(genActualType(lclType)); + init = gtNewAssignNode(lcl, zero); + } + GenTreePtr initStmt = gtNewStmt(init, callILOffset); + fgInsertStmtBefore(block, last, initStmt); } - GenTreePtr initStmt = gtNewStmt(init, callILOffset); - fgInsertStmtBefore(block, last, initStmt); } } } diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.il b/tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.il new file mode 100644 index 0000000..4bf5029 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.il @@ -0,0 +1,102 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +.assembly extern mscorlib {} + +.assembly GitHub_13822 +{ +} + + +// =============== CLASS MEMBERS DECLARATION =================== + +.class public sequential ansi sealed beforefieldinit GCStruct + extends [System.Private.CoreLib]System.ValueType +{ + .field public string str + .field public int32 i + .field public int32 j + .field public int32 k + .field public int32 l + + .method public hidebysig specialname rtspecialname + instance void .ctor(int32 i) cil managed aggressiveinlining + { + .maxstack 8 + ldarg.0 + ldstr "Hello" + stfld string GCStruct::str + ldarg.1 + brtrue.s RETURN + ldarg.0 + ldc.i4.1 + stfld int32 GCStruct::i + RETURN: ret + } // end of method GCStruct::.ctor + +} // end of class GCStruct + + // Method A has tail-recursive call that the jit turns into a loop. + // This test verifies that the temp struct returned by newobj is zero-initialized + // before calling the constructor on each iteration of the loop. + +.class public auto ansi beforefieldinit Test + extends [System.Private.CoreLib]System.Object +{ + .method public hidebysig static int32 Main() cil managed + { + .entrypoint + .maxstack 8 + ldc.i4.0 + call int32 Test::A(int32) + ret + } // end of method Test::Main + + .method public hidebysig static int32 A(int32 i) cil managed + { + .maxstack 2 + .locals init (int32 V_0) + call int32 Test::GetZero() + stloc.0 + ldarg.0 + ldc.i4.3 + blt.s L1 + ldc.i4.s 100 + ret + L1: ldarg.0 + newobj instance void GCStruct::.ctor(int32) + ldfld int32 GCStruct::i + ldarg.0 + mul + brfalse L2 + ldc.i4.m1 + ret + L2: ldarg.0 + ldc.i4.1 + add + call int32 Test::A(int32) + ret + } // end of method Test::A + + .method public hidebysig static int32 GetZero() cil managed noinlining + { + .maxstack 8 + ldc.i4.0 + ret + } // end of method Test::GetZero + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 7 (0x7) + .maxstack 8 + ldarg.0 + call instance void [System.Private.CoreLib]System.Object::.ctor() + ret + } // end of method Test::.ctor + +} // end of class Test + + +// ============================================================= diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.ilproj b/tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.ilproj new file mode 100644 index 0000000..151694e --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_13822/GitHub_13822.ilproj @@ -0,0 +1,35 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + 1 + + + + + + + False + + + + None + True + + + + + + + + + + \ No newline at end of file -- 2.7.4