From a461678ee1341d1740979f016e0158c6c0d78bf2 Mon Sep 17 00:00:00 2001 From: mikedn Date: Tue, 12 Nov 2019 20:33:18 +0200 Subject: [PATCH] Some GT_BLK/GT_OBJ related cleanup (dotnet/coreclr#27053) * Display class layout name in dumps * Delete unused gtNewBlkOpNode size parameter * gtNewObjNode always returns a GenTreeObj * Can't have GTF_VAR_DEATH on a GT_OBJ node * Delete unnecessary uses of gtNewCpObjNode * Delete unnecessary uses of gtNewBlockVal * Delete fgMorphBlkToInd Modifying nodes in place is the norm in JIT, there's no need for a function to abstract that. * Delete unused RewriteSIMDOperand keepBlk parameter * Cleanup Rationalizer::RewriteSIMDOperand Commit migrated from https://github.com/dotnet/coreclr/commit/8f9eb95bb1df95911c121fdafd65c3fe8d490a75 --- src/coreclr/src/jit/compiler.h | 5 +-- src/coreclr/src/jit/flowgraph.cpp | 9 ++--- src/coreclr/src/jit/gentree.cpp | 80 +++++++++++++++++++------------------ src/coreclr/src/jit/gschecks.cpp | 14 ++----- src/coreclr/src/jit/importer.cpp | 25 +++++------- src/coreclr/src/jit/morph.cpp | 79 ++++++++++++------------------------ src/coreclr/src/jit/objectalloc.cpp | 4 +- src/coreclr/src/jit/rationalize.cpp | 62 ++++++++++++---------------- src/coreclr/src/jit/rationalize.h | 2 +- src/coreclr/src/jit/simd.cpp | 6 +-- 10 files changed, 116 insertions(+), 170 deletions(-) diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 16920f2..ea97027 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -2502,7 +2502,7 @@ public: GenTree* gtNewSIMDVectorOne(var_types simdType, var_types baseType, unsigned size); #endif - GenTree* gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, unsigned size, bool isVolatile, bool isCopyBlock); + GenTree* gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, bool isVolatile, bool isCopyBlock); GenTree* gtNewPutArgReg(var_types type, GenTree* arg, regNumber argReg); @@ -2512,7 +2512,7 @@ protected: void gtBlockOpInit(GenTree* result, GenTree* dst, GenTree* srcOrFillVal, bool isVolatile); public: - GenTree* gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr); + GenTreeObj* gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr); void gtSetObjGcInfo(GenTreeObj* objNode); GenTree* gtNewStructVal(CORINFO_CLASS_HANDLE structHnd, GenTree* addr); GenTree* gtNewBlockVal(GenTree* addr, unsigned size); @@ -5446,7 +5446,6 @@ private: GenTree* fgMorphOneAsgBlockOp(GenTree* tree); GenTree* fgMorphInitBlock(GenTree* tree); GenTree* fgMorphPromoteLocalInitBlock(GenTreeLclVar* destLclNode, GenTree* initVal, unsigned blockSize); - GenTree* fgMorphBlkToInd(GenTreeBlk* tree, var_types type); GenTree* fgMorphGetStructAddr(GenTree** pTree, CORINFO_CLASS_HANDLE clsHnd, bool isRValue = false); GenTree* fgMorphBlkNode(GenTree* tree, bool isDest); GenTree* fgMorphBlockOperand(GenTree* tree, var_types asgType, unsigned blockWidth, bool isDest); diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index fcc2e2b..f1ddad9 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -23604,11 +23604,10 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) if (fgStructTempNeedsExplicitZeroInit(lvaTable + tmpNum, block)) { - tree = gtNewBlkOpNode(gtNewLclvNode(tmpNum, lclTyp), // Dest - gtNewIconNode(0), // Value - info.compCompHnd->getClassSize(structType), // Size - false, // isVolatile - false); // not copyBlock + tree = gtNewBlkOpNode(gtNewLclvNode(tmpNum, lclTyp), // Dest + gtNewIconNode(0), // Value + false, // isVolatile + false); // not copyBlock newStmt = gtNewStmt(tree, callILOffset); fgInsertStmtAfter(block, afterStmt, newStmt); diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp index 806cf71..bb46c8f 100644 --- a/src/coreclr/src/jit/gentree.cpp +++ b/src/coreclr/src/jit/gentree.cpp @@ -6399,27 +6399,11 @@ GenTree* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src) // Return Value: // Returns a node representing the struct value at the given address. // -// Notes: -// It will currently return a GT_OBJ node for any struct type, but may -// return a GT_IND or a non-indirection for a scalar type. - -GenTree* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr) +GenTreeObj* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr) { var_types nodeType = impNormStructType(structHnd); assert(varTypeIsStruct(nodeType)); - if (!varTypeIsStruct(nodeType)) - { - if ((addr->gtOper == GT_ADDR) && (addr->gtGetOp1()->TypeGet() == nodeType)) - { - return addr->gtGetOp1(); - } - else - { - return gtNewOperNode(GT_IND, nodeType, addr); - } - } - GenTreeObj* objNode = new (this, GT_OBJ) GenTreeObj(nodeType, addr, typGetObjLayout(structHnd)); // An Obj is not a global reference, if it is known to be a local struct. @@ -6533,19 +6517,10 @@ GenTree* Compiler::gtNewCpObjNode(GenTree* dstAddr, GenTree* srcAddr, CORINFO_CL { GenTree* lhs = gtNewStructVal(structHnd, dstAddr); GenTree* src = nullptr; - unsigned size; - if (lhs->OperIsBlk()) + if (lhs->OperIs(GT_OBJ)) { - size = lhs->AsBlk()->GetLayout()->GetSize(); - if (lhs->OperGet() == GT_OBJ) - { - gtSetObjGcInfo(lhs->AsObj()); - } - } - else - { - size = genTypeSize(lhs->gtType); + gtSetObjGcInfo(lhs->AsObj()); } if (srcAddr->OperGet() == GT_ADDR) @@ -6557,7 +6532,7 @@ GenTree* Compiler::gtNewCpObjNode(GenTree* dstAddr, GenTree* srcAddr, CORINFO_CL src = gtNewOperNode(GT_IND, lhs->TypeGet(), srcAddr); } - GenTree* result = gtNewBlkOpNode(lhs, src, size, isVolatile, true); + GenTree* result = gtNewBlkOpNode(lhs, src, isVolatile, true); return result; } @@ -6725,7 +6700,6 @@ void Compiler::gtBlockOpInit(GenTree* result, GenTree* dst, GenTree* srcOrFillVa // Arguments: // dst - Destination or target to copy to / initialize the buffer. // srcOrFillVall - the size of the buffer to copy/initialize or zero, in the case of CpObj. -// size - The size of the buffer or a class token (in the case of CpObj). // isVolatile - Whether this is a volatile memory operation or not. // isCopyBlock - True if this is a block copy (rather than a block init). // @@ -6736,7 +6710,7 @@ void Compiler::gtBlockOpInit(GenTree* result, GenTree* dst, GenTree* srcOrFillVa // If size is zero, the dst must be a GT_OBJ with the class handle. // 'dst' must be a block node or lclVar. // -GenTree* Compiler::gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, unsigned size, bool isVolatile, bool isCopyBlock) +GenTree* Compiler::gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, bool isVolatile, bool isCopyBlock) { assert(dst->OperIsBlk() || dst->OperIsLocal()); if (isCopyBlock) @@ -9500,10 +9474,6 @@ void Compiler::gtDispNodeName(GenTree* tree) { sprintf_s(bufp, sizeof(buf), " %s_ovfl%c", name, 0); } - else if (tree->OperIsBlk() && !tree->OperIsDynBlk()) - { - sprintf_s(bufp, sizeof(buf), " %s(%d)", name, tree->AsBlk()->GetLayout()->GetSize()); - } else { sprintf_s(bufp, sizeof(buf), " %s%c", name, 0); @@ -9985,6 +9955,42 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, __in __in_z _ if (tree->gtOper != GT_CAST) { printf(" %-6s", varTypeName(tree->TypeGet())); + + if (varTypeIsStruct(tree->TypeGet())) + { + ClassLayout* layout = nullptr; + + if (tree->OperIs(GT_BLK, GT_OBJ, GT_STORE_BLK, GT_STORE_OBJ)) + { + layout = tree->AsBlk()->GetLayout(); + } + else if (tree->OperIs(GT_LCL_VAR, GT_STORE_LCL_VAR)) + { + LclVarDsc* varDsc = lvaGetDesc(tree->AsLclVar()); + + if (varTypeIsStruct(varDsc->TypeGet())) + { + layout = varDsc->GetLayout(); + } + } + + if (layout != nullptr) + { + if (layout->IsBlockLayout()) + { + printf("<%u>", layout->GetSize()); + } + else if (varTypeIsSIMD(tree->TypeGet())) + { + printf("<%s>", layout->GetClassName()); + } + else + { + printf("<%s, %u>", layout->GetClassName(), layout->GetSize()); + } + } + } + if (tree->gtOper == GT_LCL_VAR || tree->gtOper == GT_STORE_LCL_VAR) { LclVarDsc* varDsc = &lvaTable[tree->AsLclVarCommon()->GetLclNum()]; @@ -10896,10 +10902,6 @@ void Compiler::gtDispTree(GenTree* tree, printf(" %s <- %s", varTypeName(toType), varTypeName(fromType)); } - if (tree->gtOper == GT_OBJ && (tree->gtFlags & GTF_VAR_DEATH)) - { - printf(" (last use)"); - } if (tree->OperIsBlkOp()) { if (tree->OperIsCopyBlkOp()) diff --git a/src/coreclr/src/jit/gschecks.cpp b/src/coreclr/src/jit/gschecks.cpp index 20ed03a..cd85658 100644 --- a/src/coreclr/src/jit/gschecks.cpp +++ b/src/coreclr/src/jit/gschecks.cpp @@ -521,16 +521,12 @@ void Compiler::gsParamsToShadows() GenTree* opAssign = nullptr; if (type == TYP_STRUCT) { - CORINFO_CLASS_HANDLE clsHnd = varDsc->lvVerTypeInfo.GetClassHandle(); - // We don't need unsafe value cls check here since we are copying the params and this flag // would have been set on the original param before reaching here. - lvaSetStruct(shadowVar, clsHnd, false); + lvaSetStruct(shadowVar, varDsc->lvVerTypeInfo.GetClassHandle(), false); - src = gtNewOperNode(GT_ADDR, TYP_BYREF, src); - dst = gtNewOperNode(GT_ADDR, TYP_BYREF, dst); + opAssign = gtNewBlkOpNode(dst, src, false, true); - opAssign = gtNewCpObjNode(dst, src, clsHnd, false); lvaTable[shadowVar].lvIsMultiRegArg = lvaTable[lclNum].lvIsMultiRegArg; lvaTable[shadowVar].lvIsMultiRegRet = lvaTable[lclNum].lvIsMultiRegRet; } @@ -579,11 +575,7 @@ void Compiler::gsParamsToShadows() GenTree* opAssign = nullptr; if (varDsc->TypeGet() == TYP_STRUCT) { - CORINFO_CLASS_HANDLE clsHnd = varDsc->lvVerTypeInfo.GetClassHandle(); - src = gtNewOperNode(GT_ADDR, TYP_BYREF, src); - dst = gtNewOperNode(GT_ADDR, TYP_BYREF, dst); - - opAssign = gtNewCpObjNode(dst, src, clsHnd, false); + opAssign = gtNewBlkOpNode(dst, src, false, true); } else { diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 2cd2a7d..0558622 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -3364,15 +3364,14 @@ GenTree* Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig) dataOffset = eeGetArrayDataOffset(elementType); } - GenTree* dst = gtNewOperNode(GT_ADD, TYP_BYREF, arrayLocalNode, gtNewIconNode(dataOffset, TYP_I_IMPL)); - GenTree* blk = gtNewBlockVal(dst, blkSize); - GenTree* src = gtNewIndOfIconHandleNode(TYP_STRUCT, (size_t)initData, GTF_ICON_STATIC_HDL, false); - - return gtNewBlkOpNode(blk, // dst - src, // src - blkSize, // size - false, // volatil - true); // copyBlock + GenTree* dstAddr = gtNewOperNode(GT_ADD, TYP_BYREF, arrayLocalNode, gtNewIconNode(dataOffset, TYP_I_IMPL)); + GenTree* dst = new (this, GT_BLK) GenTreeBlk(GT_BLK, TYP_STRUCT, dstAddr, typGetBlkLayout(blkSize)); + GenTree* src = gtNewIndOfIconHandleNode(TYP_STRUCT, (size_t)initData, GTF_ICON_STATIC_HDL, false); + + return gtNewBlkOpNode(dst, // dst + src, // src + false, // volatile + true); // copyBlock } //------------------------------------------------------------------------ @@ -13669,7 +13668,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) } CorInfoType jitTyp = info.compCompHnd->asCorInfoType(resolvedToken.hClass); - unsigned size = info.compCompHnd->getClassSize(resolvedToken.hClass); if (impIsPrimitive(jitTyp)) { @@ -13689,7 +13687,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) newObjThisPtr = gtNewBlkOpNode(newObjThisPtr, // Dest gtNewIconNode(0), // Value - size, // Size false, // isVolatile false); // not copyBlock impAppendTree(newObjThisPtr, (unsigned)CHECK_SPILL_NONE, impCurStmtOffs); @@ -15630,7 +15627,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = gtNewIconNode(0); // Value op1 = impPopStack().val; // Dest op1 = gtNewBlockVal(op1, size); - op1 = gtNewBlkOpNode(op1, op2, size, (prefixFlags & PREFIX_VOLATILE) != 0, false); + op1 = gtNewBlkOpNode(op1, op2, (prefixFlags & PREFIX_VOLATILE) != 0, false); goto SPILL_APPEND; case CEE_INITBLK: @@ -15654,7 +15651,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = new (this, GT_DYN_BLK) GenTreeDynBlk(op1, op3); size = 0; } - op1 = gtNewBlkOpNode(op1, op2, size, (prefixFlags & PREFIX_VOLATILE) != 0, false); + op1 = gtNewBlkOpNode(op1, op2, (prefixFlags & PREFIX_VOLATILE) != 0, false); goto SPILL_APPEND; @@ -15687,7 +15684,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = gtNewOperNode(GT_IND, TYP_STRUCT, op2); } - op1 = gtNewBlkOpNode(op1, op2, size, (prefixFlags & PREFIX_VOLATILE) != 0, true); + op1 = gtNewBlkOpNode(op1, op2, (prefixFlags & PREFIX_VOLATILE) != 0, true); goto SPILL_APPEND; case CEE_CPOBJ: diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 94c2543..3f73f60 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -4193,12 +4193,10 @@ void Compiler::fgMorphMultiregStructArgs(GenTreeCall* call) { if (argx->OperIs(GT_OBJ)) { - fgMorphBlkToInd(argx->AsObj(), hfaType); - } - else - { - argx->gtType = hfaType; + argx->SetOper(GT_IND); } + + argx->gtType = hfaType; } } @@ -4952,8 +4950,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, } // Copy the valuetype to the temp - unsigned size = info.compCompHnd->getClassSize(copyBlkClass); - GenTree* copyBlk = gtNewBlkOpNode(dest, argx, size, false /* not volatile */, true /* copyBlock */); + GenTree* copyBlk = gtNewBlkOpNode(dest, argx, false /* not volatile */, true /* copyBlock */); copyBlk = fgMorphCopyBlock(copyBlk); #if FEATURE_FIXED_OUT_ARGS @@ -5767,7 +5764,7 @@ GenTree* Compiler::fgMorphStackArgForVarArgs(unsigned lclNum, var_types varType, GenTree* tree; if (varTypeIsStruct(varType)) { - tree = gtNewBlockVal(ptrArg, varDsc->lvExactSize); + tree = new (this, GT_BLK) GenTreeBlk(GT_BLK, TYP_STRUCT, ptrArg, typGetBlkLayout(varDsc->lvExactSize)); } else { @@ -5813,7 +5810,7 @@ GenTree* Compiler::fgMorphLocalVar(GenTree* tree, bool forceRemorph) { if (newTree->OperIsBlk() && ((tree->gtFlags & GTF_VAR_DEF) == 0)) { - fgMorphBlkToInd(newTree->AsBlk(), newTree->gtType); + newTree->SetOper(GT_IND); } return newTree; } @@ -8008,8 +8005,8 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa { const bool isVolatile = false; const bool isCopyBlock = false; - init = gtNewBlkOpNode(lcl, gtNewIconNode(0), varDsc->lvSize(), isVolatile, isCopyBlock); - init = fgMorphInitBlock(init); + init = gtNewBlkOpNode(lcl, gtNewIconNode(0), isVolatile, isCopyBlock); + init = fgMorphInitBlock(init); } else { @@ -8562,7 +8559,7 @@ GenTree* Compiler::fgMorphLeaf(GenTree* tree) { if (newTree->OperIsBlk() && ((tree->gtFlags & GTF_VAR_DEF) == 0)) { - fgMorphBlkToInd(newTree->AsBlk(), newTree->gtType); + newTree->SetOper(GT_IND); } return newTree; } @@ -9349,27 +9346,6 @@ GenTree* Compiler::fgMorphPromoteLocalInitBlock(GenTreeLclVar* destLclNode, GenT } //------------------------------------------------------------------------ -// fgMorphBlkToInd: Change a blk node into a GT_IND of the specified type -// -// Arguments: -// tree - the node to be modified. -// type - the type of indirection to change it to. -// -// Return Value: -// Returns the node, modified in place. -// -// Notes: -// This doesn't really warrant a separate method, but is here to abstract -// the fact that these nodes can be modified in-place. - -GenTree* Compiler::fgMorphBlkToInd(GenTreeBlk* tree, var_types type) -{ - tree->SetOper(GT_IND); - tree->gtType = type; - return tree; -} - -//------------------------------------------------------------------------ // fgMorphGetStructAddr: Gets the address of a struct object // // Arguments: @@ -9461,8 +9437,6 @@ GenTree* Compiler::fgMorphBlkNode(GenTree* tree, bool isDest) // The pattern is that the COMMA should be the address expression. // Therefore, we insert a GT_ADDR just above the node, and wrap it in an obj or ind. // TODO-1stClassStructs: Consider whether this can be improved. - // Also consider whether some of this can be included in gtNewBlockVal (though note - // that doing so may cause us to query the type system before we otherwise would). // Example: // before: [3] comma struct <- [2] comma struct <- [1] LCL_VAR struct // after: [3] comma byref <- [2] comma byref <- [4] addr byref <- [1] LCL_VAR struct @@ -9512,10 +9486,7 @@ GenTree* Compiler::fgMorphBlkNode(GenTree* tree, bool isDest) else { tree = gtNewObjNode(structHnd, addr); - if (tree->OperGet() == GT_OBJ) - { - gtSetObjGcInfo(tree->AsObj()); - } + gtSetObjGcInfo(tree->AsObj()); } } else @@ -9606,7 +9577,7 @@ GenTree* Compiler::fgMorphBlockOperand(GenTree* tree, var_types asgType, unsigne } else if (effectiveVal->OperIsBlk()) { - effectiveVal = fgMorphBlkToInd(effectiveVal->AsBlk(), asgType); + effectiveVal->SetOper(GT_IND); } } effectiveVal->gtType = asgType; @@ -9660,7 +9631,8 @@ GenTree* Compiler::fgMorphBlockOperand(GenTree* tree, var_types asgType, unsigne { if (indirTree->OperIsBlk() && !isBlkReqd) { - (void)fgMorphBlkToInd(effectiveVal->AsBlk(), asgType); + effectiveVal->SetOper(GT_IND); + effectiveVal->gtType = asgType; } else { @@ -9682,10 +9654,7 @@ GenTree* Compiler::fgMorphBlockOperand(GenTree* tree, var_types asgType, unsigne else { newTree = gtNewObjNode(clsHnd, addr); - if (newTree->OperGet() == GT_OBJ) - { - gtSetObjGcInfo(newTree->AsObj()); - } + gtSetObjGcInfo(newTree->AsObj()); } } else @@ -10176,7 +10145,8 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) dest = fgMorphBlockOperand(dest, asgType, blockWidth, false /*isBlkReqd*/); if (dest->OperIsBlk()) { - (void)fgMorphBlkToInd(dest->AsBlk(), TYP_STRUCT); + dest->SetOper(GT_IND); + dest->gtType = TYP_STRUCT; } destAddr = gtNewOperNode(GT_ADDR, TYP_BYREF, dest); } @@ -10836,7 +10806,8 @@ GenTree* Compiler::fgMorphFieldAssignToSIMDIntrinsicSet(GenTree* tree) if (tree->gtGetOp1()->OperIsBlk()) { assert(tree->gtGetOp1()->TypeGet() == simdType); - fgMorphBlkToInd(tree->gtGetOp1()->AsBlk(), simdType); + tree->gtGetOp1()->SetOper(GT_IND); + tree->gtGetOp1()->gtType = simdType; } } #ifdef DEBUG @@ -13253,7 +13224,7 @@ DONE_MORPHING_CHILDREN: GenTree* commaOp2 = commaNode->AsOp()->gtOp2; if (commaOp2->OperIsBlk()) { - commaOp2 = fgMorphBlkToInd(commaOp2->AsBlk(), commaOp2->TypeGet()); + commaOp2->SetOper(GT_IND); } if (commaOp2->gtOper == GT_IND) { @@ -17152,7 +17123,7 @@ void Compiler::fgRetypeImplicitByRefArgs() if (lvaIsImplicitByRefLocal(lclNum)) { - size_t size; + unsigned size; if (varDsc->lvSize() > REGSIZE_BYTES) { @@ -17216,7 +17187,7 @@ void Compiler::fgRetypeImplicitByRefArgs() GenTree* lhs = gtNewLclvNode(newLclNum, varDsc->lvType); // RHS is an indirection (using GT_OBJ) off the parameter. GenTree* addr = gtNewLclvNode(lclNum, TYP_BYREF); - GenTree* rhs = gtNewBlockVal(addr, (unsigned)size); + GenTree* rhs = new (this, GT_BLK) GenTreeBlk(GT_BLK, TYP_STRUCT, addr, typGetBlkLayout(size)); GenTree* assign = gtNewAssignNode(lhs, rhs); fgNewStmtAtBeg(fgFirstBB, assign); } @@ -17537,11 +17508,11 @@ GenTree* Compiler::fgMorphImplicitByRefArgs(GenTree* tree, bool isAddr) else { tree = gtNewObjNode(lclVarDsc->lvVerTypeInfo.GetClassHandle(), tree); - } - if (structType == TYP_STRUCT) - { - gtSetObjGcInfo(tree->AsObj()); + if (structType == TYP_STRUCT) + { + gtSetObjGcInfo(tree->AsObj()); + } } // TODO-CQ: If the VM ever stops violating the ABI and passing heap references diff --git a/src/coreclr/src/jit/objectalloc.cpp b/src/coreclr/src/jit/objectalloc.cpp index fe04380..e5cccf2 100644 --- a/src/coreclr/src/jit/objectalloc.cpp +++ b/src/coreclr/src/jit/objectalloc.cpp @@ -513,8 +513,6 @@ unsigned int ObjectAllocator::MorphAllocObjNodeIntoStackAlloc(GenTreeAllocObj* a // Initialize the object memory if necessary if (comp->fgStructTempNeedsExplicitZeroInit(comp->lvaTable + lclNum, block)) { - unsigned int structSize = comp->lvaTable[lclNum].lvSize(); - //------------------------------------------------------------------------ // STMTx (IL 0x... ???) // * ASG struct (init) @@ -525,7 +523,7 @@ unsigned int ObjectAllocator::MorphAllocObjNodeIntoStackAlloc(GenTreeAllocObj* a GenTree* tree = comp->gtNewLclvNode(lclNum, TYP_STRUCT); const bool isVolatile = false; const bool isCopyBlock = false; - tree = comp->gtNewBlkOpNode(tree, comp->gtNewIconNode(0), structSize, isVolatile, isCopyBlock); + tree = comp->gtNewBlkOpNode(tree, comp->gtNewIconNode(0), isVolatile, isCopyBlock); Statement* newStmt = comp->gtNewStmt(tree); diff --git a/src/coreclr/src/jit/rationalize.cpp b/src/coreclr/src/jit/rationalize.cpp index 4c1f32c..089d7fb 100644 --- a/src/coreclr/src/jit/rationalize.cpp +++ b/src/coreclr/src/jit/rationalize.cpp @@ -44,20 +44,15 @@ void copyFlags(GenTree* dst, GenTree* src, unsigned mask) dst->gtFlags |= (src->gtFlags & mask); } -// Rewrite a SIMD indirection as GT_IND(GT_LEA(obj.op1)), or as a simple -// lclVar if possible. +// RewriteSIMDIndir: Rewrite a SIMD indirection as a simple lclVar if possible. // // Arguments: -// use - A use reference for a block node -// keepBlk - True if this should remain a block node if it is not a lclVar -// -// Return Value: -// None. +// use - A use of a GT_IND node of SIMD type // // TODO-1stClassStructs: These should be eliminated earlier, once we can handle // lclVars in all the places that used to have GT_OBJ. // -void Rationalizer::RewriteSIMDOperand(LIR::Use& use, bool keepBlk) +void Rationalizer::RewriteSIMDIndir(LIR::Use& use) { #ifdef FEATURE_SIMD // No lowering is needed for non-SIMD nodes, so early out if SIMD types are not supported. @@ -66,24 +61,18 @@ void Rationalizer::RewriteSIMDOperand(LIR::Use& use, bool keepBlk) return; } - GenTree* tree = use.Def(); - if (!tree->OperIsIndir()) - { - return; - } - var_types simdType = tree->TypeGet(); + GenTreeIndir* indir = use.Def()->AsIndir(); + assert(indir->OperIs(GT_IND)); + var_types simdType = indir->TypeGet(); + assert(varTypeIsSIMD(simdType)); - if (!varTypeIsSIMD(simdType)) - { - return; - } + GenTree* addr = indir->Addr(); - // If we have GT_IND(GT_LCL_VAR_ADDR) and the var is a SIMD type, - // replace the expression by GT_LCL_VAR or GT_LCL_FLD. - GenTree* addr = tree->AsIndir()->Addr(); if (addr->OperIs(GT_LCL_VAR_ADDR) && comp->lvaGetDesc(addr->AsLclVar())->lvSIMDType) { - BlockRange().Remove(tree); + // If we have GT_IND(GT_LCL_VAR_ADDR) and the var is a SIMD type, + // replace the expression by GT_LCL_VAR or GT_LCL_FLD. + BlockRange().Remove(indir); var_types lclType = comp->lvaGetDesc(addr->AsLclVar())->TypeGet(); @@ -108,18 +97,17 @@ void Rationalizer::RewriteSIMDOperand(LIR::Use& use, bool keepBlk) addr->gtType = simdType; use.ReplaceWith(comp, addr); } - else if ((addr->OperGet() == GT_ADDR) && (addr->gtGetOp1()->OperIsSimdOrHWintrinsic())) + else if (addr->OperIs(GT_ADDR) && addr->AsUnOp()->gtGetOp1()->OperIsSimdOrHWintrinsic()) { - // if we have GT_IND(GT_ADDR(GT_SIMD)), remove the GT_IND(GT_ADDR()), leaving just the GT_SIMD. - BlockRange().Remove(tree); + // If we have IND(ADDR(SIMD)) then we can keep only the SIMD node. + // This is a special tree created by impNormStructVal to preserve the class layout + // needed by call morphing on an OBJ node. This information is no longer needed at + // this point (and the address of a SIMD node can't be obtained anyway). + + BlockRange().Remove(indir); BlockRange().Remove(addr); - use.ReplaceWith(comp, addr->gtGetOp1()); - } - else if (!keepBlk) - { - tree->SetOper(GT_IND); - tree->gtType = simdType; + use.ReplaceWith(comp, addr->AsUnOp()->gtGetOp1()); } #endif // FEATURE_SIMD } @@ -380,7 +368,7 @@ void Rationalizer::RewriteAssignment(LIR::Use& use) if (varDsc->HasGCPtr()) { CORINFO_CLASS_HANDLE structHnd = varDsc->lvVerTypeInfo.GetClassHandle(); - GenTreeObj* objNode = comp->gtNewObjNode(structHnd, location)->AsObj(); + GenTreeObj* objNode = comp->gtNewObjNode(structHnd, location); objNode->ChangeOper(GT_STORE_OBJ); objNode->SetData(value); storeBlk = objNode; @@ -616,7 +604,7 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge if (varTypeIsSIMD(node)) { - RewriteSIMDOperand(use, false); + RewriteSIMDIndir(use); } else { @@ -757,11 +745,11 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge break; case GT_OBJ: - assert(node->TypeGet() == TYP_STRUCT || !use.User()->OperIsInitBlkOp()); - if (varTypeIsSIMD(node)) + assert((node->TypeGet() == TYP_STRUCT) || !use.User()->OperIsInitBlkOp()); + if (varTypeIsSIMD(node->TypeGet())) { - // Rewrite these as GT_IND. - RewriteSIMDOperand(use, false); + node->SetOper(GT_IND); + RewriteSIMDIndir(use); } break; diff --git a/src/coreclr/src/jit/rationalize.h b/src/coreclr/src/jit/rationalize.h index 305d73e..d396ac8 100644 --- a/src/coreclr/src/jit/rationalize.h +++ b/src/coreclr/src/jit/rationalize.h @@ -36,7 +36,7 @@ private: } // SIMD related - void RewriteSIMDOperand(LIR::Use& use, bool keepBlk); + void RewriteSIMDIndir(LIR::Use& use); // Intrinsic related transformations void RewriteNodeAsCall(GenTree** use, diff --git a/src/coreclr/src/jit/simd.cpp b/src/coreclr/src/jit/simd.cpp index 67aa0d6..810b57e 100644 --- a/src/coreclr/src/jit/simd.cpp +++ b/src/coreclr/src/jit/simd.cpp @@ -3132,7 +3132,7 @@ GenTree* Compiler::impSIMDIntrinsic(OPCODE opcode, // GT_OBJ instead of GT_BLK nodes to avoid losing information about the actual vector type. GenTree* loDest = new (this, GT_BLK) GenTreeBlk(GT_BLK, simdType, dstAddrLo, typGetBlkLayout(getSIMDTypeSizeInBytes(clsHnd))); - GenTree* loAsg = gtNewBlkOpNode(loDest, simdTree, getSIMDTypeSizeInBytes(clsHnd), + GenTree* loAsg = gtNewBlkOpNode(loDest, simdTree, false, // not volatile true); // copyBlock loAsg->gtFlags |= ((simdTree->gtFlags | dstAddrLo->gtFlags) & GTF_ALL_EFFECT); @@ -3141,7 +3141,7 @@ GenTree* Compiler::impSIMDIntrinsic(OPCODE opcode, simdTree = gtNewSIMDNode(simdType, dupOp1, nullptr, SIMDIntrinsicWidenHi, baseType, size); GenTree* hiDest = new (this, GT_BLK) GenTreeBlk(GT_BLK, simdType, dstAddrHi, typGetBlkLayout(getSIMDTypeSizeInBytes(clsHnd))); - GenTree* hiAsg = gtNewBlkOpNode(hiDest, simdTree, getSIMDTypeSizeInBytes(clsHnd), + GenTree* hiAsg = gtNewBlkOpNode(hiDest, simdTree, false, // not volatile true); // copyBlock hiAsg->gtFlags |= ((simdTree->gtFlags | dstAddrHi->gtFlags) & GTF_ALL_EFFECT); @@ -3181,7 +3181,7 @@ GenTree* Compiler::impSIMDIntrinsic(OPCODE opcode, GenTree* dest = new (this, GT_BLK) GenTreeBlk(GT_BLK, simdType, copyBlkDst, typGetBlkLayout(getSIMDTypeSizeInBytes(clsHnd))); dest->gtFlags |= GTF_GLOB_REF; - retVal = gtNewBlkOpNode(dest, simdTree, getSIMDTypeSizeInBytes(clsHnd), + retVal = gtNewBlkOpNode(dest, simdTree, false, // not volatile true); // copyBlock retVal->gtFlags |= ((simdTree->gtFlags | copyBlkDst->gtFlags) & GTF_ALL_EFFECT); -- 2.7.4