From 2ac55d44c2b0222476690579bd866efa93c98048 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Tue, 16 Aug 2016 13:42:50 -0700 Subject: [PATCH] Preparatory changes for Blk Ops IR These are mostly refactoring changes, in preparation for the change to the IR for block assignments. Commit migrated from https://github.com/dotnet/coreclr/commit/cccff234505abfbc0e5e030180d988106b852326 --- src/coreclr/src/jit/assertionprop.cpp | 31 ++- src/coreclr/src/jit/codegencommon.cpp | 10 +- src/coreclr/src/jit/codegenlegacy.cpp | 75 +++--- src/coreclr/src/jit/codegenxarch.cpp | 13 +- src/coreclr/src/jit/earlyprop.cpp | 6 +- src/coreclr/src/jit/gentree.h | 24 ++ src/coreclr/src/jit/importer.cpp | 31 ++- src/coreclr/src/jit/lower.cpp | 7 +- src/coreclr/src/jit/morph.cpp | 442 +++++++++++++++------------------- src/coreclr/src/jit/rationalize.cpp | 10 +- src/coreclr/src/jit/ssabuilder.cpp | 2 +- src/coreclr/src/jit/valuenum.cpp | 46 ++-- 12 files changed, 366 insertions(+), 331 deletions(-) diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index a85f731..fdcf8da 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -1100,6 +1100,11 @@ Compiler::AssertionIndex Compiler::optCreateAssertion(GenTreePtr op1, CNS_COMMON: { + // TODO-1stClassStructs: handle constant propagation to struct types. + if (varTypeIsStruct(lclVar)) + { + goto DONE_ASSERTION; + } // // Must either be an OAK_EQUAL or an OAK_NOT_EQUAL assertion // @@ -2029,7 +2034,12 @@ void Compiler::optAssertionGen(GenTreePtr tree) { case GT_ASG: // VN takes care of non local assertions for assignments and data flow. - if (optLocalAssertionProp) + // TODO-1stClassStructs: Enable assertion prop for struct types. + if (varTypeIsStruct(tree)) + { + // Do nothing. + } + else if (optLocalAssertionProp) { assertionIndex = optCreateAssertion(tree->gtOp.gtOp1, tree->gtOp.gtOp2, OAK_EQUAL); } @@ -2040,8 +2050,18 @@ void Compiler::optAssertionGen(GenTreePtr tree) break; case GT_IND: + // TODO-1stClassStructs: All indirections should be considered to create a non-null + // assertion, but previously, when these indirections were implicit due to a block + // copy or init, they were not being considered to do so. + if (tree->gtType == TYP_STRUCT) + { + GenTree* parent = tree->gtGetParent(nullptr); + if ((parent != nullptr) && (parent->gtOper == GT_ASG)) + { + break; + } + } case GT_NULLCHECK: - // An indirection can create a non-null assertion case GT_ARR_LENGTH: // An array length can create a non-null assertion assertionIndex = optCreateAssertion(tree->gtOp.gtOp1, nullptr, OAK_NOT_EQUAL); @@ -4221,7 +4241,7 @@ void Compiler::optImpliedByCopyAssertion(AssertionDsc* copyAssertion, AssertionD break; case O2K_IND_CNS_INT: - // This is the ngen case where we have a GT_IND of an address. + // This is the ngen case where we have an indirection of an address. noway_assert((impAssertion->op1.kind == O1K_EXACT_TYPE) || (impAssertion->op1.kind == O1K_SUBTYPE)); __fallthrough; @@ -4784,7 +4804,7 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, Gen // // Description: // Performs value number based non-null propagation on GT_CALL and -// GT_IND/GT_NULLCHECK. This is different from flow based assertions and helps +// indirections. This is different from flow based assertions and helps // unify VN based constant prop and non-null prop in a single pre-order walk. // void Compiler::optVnNonNullPropCurStmt(BasicBlock* block, GenTreePtr stmt, GenTreePtr tree) @@ -4797,6 +4817,9 @@ void Compiler::optVnNonNullPropCurStmt(BasicBlock* block, GenTreePtr stmt, GenTr } else if (tree->OperGet() == GT_IND || tree->OperGet() == GT_NULLCHECK) { + // TODO-1stClassStructs: All indirections should be handled here, but + // previously, when these indirections were GT_OBJ, or implicit due to a block + // copy or init, they were not being handled. newTree = optAssertionProp_Ind(empty, tree, stmt); } if (newTree) diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index 3ca2fc5..0bb8efc 100755 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -724,17 +724,13 @@ void Compiler::compUpdateLifeVar(GenTreePtr tree, VARSET_TP* pLastUseVars) LclVarDsc* varDsc = lvaTable + lclNum; #ifdef DEBUG -#if !defined(_TARGET_AMD64_) // no addr nodes on AMD and experimenting with with encountering vars in 'random' order +#if !defined(_TARGET_AMD64_) + // There are no addr nodes on ARM and we are experimenting with encountering vars in 'random' order. // Struct fields are not traversed in a consistent order, so ignore them when // verifying that we see the var nodes in execution order if (ForCodeGen) { - if (tree->gtOper == GT_OBJ) - { - // The tree must have the particular form OBJ(ADDR(LCL)); no need to do the check below. - assert(indirAddrLocal != NULL); - } - else if (tree->OperIsIndir()) + if (tree->OperIsIndir()) { assert(indirAddrLocal != NULL); } diff --git a/src/coreclr/src/jit/codegenlegacy.cpp b/src/coreclr/src/jit/codegenlegacy.cpp index 4351f98..4ef4f6e 100644 --- a/src/coreclr/src/jit/codegenlegacy.cpp +++ b/src/coreclr/src/jit/codegenlegacy.cpp @@ -8991,7 +8991,10 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) GenTreePtr opsPtr[3]; regMaskTP regsPtr[3]; - noway_assert(oper == GT_COPYBLK || oper == GT_INITBLK); + const bool isCopyBlk = (oper == GT_COPYBLK); + const bool isInitBlk = (oper == GT_INITBLK); + const bool sizeIsConst = op2->IsCnsIntOrI(); + noway_assert(isCopyBlk || isInitBlk); noway_assert(op1->IsList()); #ifdef _TARGET_ARM_ @@ -9006,15 +9009,15 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) destPtr = op1->gtOp.gtOp1; srcPtrOrVal = op1->gtOp.gtOp2; noway_assert(destPtr->TypeGet() == TYP_BYREF || varTypeIsIntegral(destPtr->TypeGet())); - noway_assert((oper == GT_COPYBLK && + noway_assert((isCopyBlk && (srcPtrOrVal->TypeGet() == TYP_BYREF || varTypeIsIntegral(srcPtrOrVal->TypeGet()))) || - (oper == GT_INITBLK && varTypeIsIntegral(srcPtrOrVal->TypeGet()))); + (isInitBlk && varTypeIsIntegral(srcPtrOrVal->TypeGet()))); noway_assert(op1 && op1->IsList()); noway_assert(destPtr && srcPtrOrVal); #if CPU_USES_BLOCK_MOVE - regs = (oper == GT_INITBLK) ? RBM_EAX : RBM_ESI; // What is the needReg for Val/Src + regs = isInitBlk ? RBM_EAX : RBM_ESI; // What is the needReg for Val/Src /* Some special code for block moves/inits for constant sizes */ @@ -9022,20 +9025,20 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) // Is this a fixed size COPYBLK? // or a fixed size INITBLK with a constant init value? // - if ((op2->IsCnsIntOrI()) && ((oper == GT_COPYBLK) || (srcPtrOrVal->IsCnsIntOrI()))) + if ((sizeIsConst) && (isCopyBlk || (srcPtrOrVal->IsCnsIntOrI()))) { size_t length = (size_t)op2->gtIntCon.gtIconVal; size_t initVal = 0; instruction ins_P, ins_PR, ins_B; - if (oper == GT_INITBLK) + if (isInitBlk) { ins_P = INS_stosp; ins_PR = INS_r_stosp; ins_B = INS_stosb; /* Properly extend the init constant from a U1 to a U4 */ - initVal = 0xFF & ((unsigned)op1->gtOp.gtOp2->gtIntCon.gtIconVal); + initVal = 0xFF & ((unsigned)srcPtrOrVal->gtIntCon.gtIconVal); /* If it is a non-zero value we have to replicate */ /* the byte value four times to form the DWORD */ @@ -9052,11 +9055,11 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) } else { - op1->gtOp.gtOp2->gtType = TYP_INT; + srcPtrOrVal->gtType = TYP_INT; } #endif // _TARGET_64BIT_ } - op1->gtOp.gtOp2->gtIntCon.gtIconVal = initVal; + srcPtrOrVal->gtIntCon.gtIconVal = initVal; } else { @@ -9102,7 +9105,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) movqLenMax = 48; } - if ((compiler->compCodeOpt() == Compiler::FAST_CODE) || (oper == GT_INITBLK)) + if ((compiler->compCodeOpt() == Compiler::FAST_CODE) || isInitBlk) { // Be more aggressive when optimizing for speed // InitBlk uses fewer instructions @@ -9116,7 +9119,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) if ((length % 8) == 0) { bWillUseOnlySSE2 = true; - if (oper == GT_INITBLK && (initVal == 0)) + if (isInitBlk && (initVal == 0)) { bNeedEvaluateCnst = false; noway_assert((op1->gtOp.gtOp2->OperGet() == GT_CNS_INT)); @@ -9127,7 +9130,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) #endif // !_TARGET_64BIT_ - const bool bWillTrashRegSrc = ((oper == GT_COPYBLK) && !bWillUseOnlySSE2); + const bool bWillTrashRegSrc = (isCopyBlk && !bWillUseOnlySSE2); /* Evaluate dest and src/val */ if (op1->gtFlags & GTF_REVERSE_OPS) @@ -9160,7 +9163,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) int blkDisp = 0; regNumber xmmReg = REG_XMM0; - if (oper == GT_INITBLK) + if (isInitBlk) { if (initVal) { @@ -9175,11 +9178,11 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) JITLOG_THIS(compiler, (LL_INFO100, "Using XMM instructions for %3d byte %s while compiling %s\n", length, - (oper == GT_INITBLK) ? "initblk" : "copyblk", compiler->info.compFullName)); + isInitBlk ? "initblk" : "copyblk", compiler->info.compFullName)); while (length > 7) { - if (oper == GT_INITBLK) + if (isInitBlk) { getEmitter()->emitIns_AR_R(INS_movq, EA_8BYTE, xmmReg, REG_EDI, blkDisp); } @@ -9197,7 +9200,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) noway_assert(bNeedEvaluateCnst); noway_assert(!bWillUseOnlySSE2); - if (oper == GT_COPYBLK) + if (isCopyBlk) { inst_RV_IV(INS_add, REG_ESI, blkDisp, emitActualTypeSize(srcPtrOrVal->TypeGet())); bTrashedESI = true; @@ -9234,7 +9237,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) } bTrashedEDI = true; - if (oper == GT_COPYBLK) + if (isCopyBlk) bTrashedESI = true; } else @@ -9252,7 +9255,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) regTracker.rsTrackRegTrash(REG_ECX); bTrashedEDI = true; - if (oper == GT_COPYBLK) + if (isCopyBlk) bTrashedESI = true; } @@ -9265,11 +9268,11 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) noway_assert(bNeedEvaluateCnst); noway_assert(length < 8); - instGen((oper == GT_INITBLK) ? INS_stosd : INS_movsd); + instGen((isInitBlk) ? INS_stosd : INS_movsd); length -= 4; bTrashedEDI = true; - if (oper == GT_COPYBLK) + if (isCopyBlk) bTrashedESI = true; } @@ -9285,7 +9288,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) } bTrashedEDI = true; - if (oper == GT_COPYBLK) + if (isCopyBlk) bTrashedESI = true; } @@ -9311,7 +9314,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) compiler->fgOrderBlockOps(tree, RBM_EDI, regs, RBM_ECX, opsPtr, regsPtr); // OUT arguments - noway_assert(((oper == GT_INITBLK) && (regs == RBM_EAX)) || ((oper == GT_COPYBLK) && (regs == RBM_ESI))); + noway_assert((isInitBlk && (regs == RBM_EAX)) || (isCopyBlk && (regs == RBM_ESI))); genComputeReg(opsPtr[0], regsPtr[0], RegSet::EXACT_REG, RegSet::KEEP_REG, (regsPtr[0] != RBM_EAX)); genComputeReg(opsPtr[1], regsPtr[1], RegSet::EXACT_REG, RegSet::KEEP_REG, (regsPtr[1] != RBM_EAX)); genComputeReg(opsPtr[2], regsPtr[2], RegSet::EXACT_REG, RegSet::KEEP_REG, (regsPtr[2] != RBM_EAX)); @@ -9328,7 +9331,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) noway_assert((op2->gtFlags & GTF_REG_VAL) && // Size (op2->gtRegNum == REG_ECX)); - if (oper == GT_INITBLK) + if (isInitBlk) instGen(INS_r_stosb); else instGen(INS_r_movsb); @@ -9336,7 +9339,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) regTracker.rsTrackRegTrash(REG_EDI); regTracker.rsTrackRegTrash(REG_ECX); - if (oper == GT_COPYBLK) + if (isCopyBlk) regTracker.rsTrackRegTrash(REG_ESI); // else No need to trash EAX as it wasnt destroyed by the "rep stos" @@ -9355,7 +9358,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) // Is this a fixed size COPYBLK? // or a fixed size INITBLK with a constant init value? // - if ((op2->OperGet() == GT_CNS_INT) && ((oper == GT_COPYBLK) || (srcPtrOrVal->OperGet() == GT_CNS_INT))) + if ((op2->OperGet() == GT_CNS_INT) && (isCopyBlk || (srcPtrOrVal->OperGet() == GT_CNS_INT))) { GenTreePtr dstOp = op1->gtOp.gtOp1; GenTreePtr srcOp = op1->gtOp.gtOp2; @@ -9364,7 +9367,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) unsigned initVal = 0; bool useLoop = false; - if (oper == GT_INITBLK) + if (isInitBlk) { /* Properly extend the init constant from a U1 to a U4 */ initVal = 0xFF & ((unsigned)srcOp->gtIntCon.gtIconVal); @@ -9381,7 +9384,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) } // Will we be using a loop to implement this INITBLK/COPYBLK? - if (((oper == GT_COPYBLK) && (fullStoreCount >= 8)) || ((oper == GT_INITBLK) && (fullStoreCount >= 16))) + if ((isCopyBlk && (fullStoreCount >= 8)) || (isInitBlk && (fullStoreCount >= 16))) { useLoop = true; } @@ -9427,7 +9430,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) emitAttr dstType = (varTypeIsGC(dstOp) && !dstIsOnStack) ? EA_BYREF : EA_PTRSIZE; emitAttr srcType; - if (oper == GT_COPYBLK) + if (isCopyBlk) { // Prefer a low register,but avoid one of the ones we've already grabbed regTemp = regSet.rsGrabReg(regSet.rsNarrowHint(regSet.rsRegMaskCanGrab() & ~usedRegs, RBM_LOW_REGS)); @@ -9451,7 +9454,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) { for (unsigned i = 0; i < fullStoreCount; i++) { - if (oper == GT_COPYBLK) + if (isCopyBlk) { getEmitter()->emitIns_R_R_I(loadIns, EA_4BYTE, regTemp, regSrc, i * TARGET_POINTER_SIZE); getEmitter()->emitIns_R_R_I(storeIns, EA_4BYTE, regTemp, regDst, i * TARGET_POINTER_SIZE); @@ -9473,7 +9476,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) // We need a second temp register for CopyBlk regNumber regTemp2 = REG_STK; - if (oper == GT_COPYBLK) + if (isCopyBlk) { // Prefer a low register, but avoid one of the ones we've already grabbed regTemp2 = @@ -9492,7 +9495,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) genDefineTempLabel(loopTopBlock); // The loop body - if (oper == GT_COPYBLK) + if (isCopyBlk) { getEmitter()->emitIns_R_R_I(loadIns, EA_4BYTE, regTemp, regSrc, 0); getEmitter()->emitIns_R_R_I(loadIns, EA_4BYTE, regTemp2, regSrc, TARGET_POINTER_SIZE); @@ -9523,7 +9526,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) if (length & TARGET_POINTER_SIZE) { - if (oper == GT_COPYBLK) + if (isCopyBlk) { getEmitter()->emitIns_R_R_I(loadIns, EA_4BYTE, regTemp, regSrc, 0); getEmitter()->emitIns_R_R_I(storeIns, EA_4BYTE, regTemp, regDst, 0); @@ -9546,7 +9549,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) loadIns = ins_Load(TYP_USHORT); // INS_ldrh storeIns = ins_Store(TYP_USHORT); // INS_strh - if (oper == GT_COPYBLK) + if (isCopyBlk) { getEmitter()->emitIns_R_R_I(loadIns, EA_2BYTE, regTemp, regSrc, finalOffset); getEmitter()->emitIns_R_R_I(storeIns, EA_2BYTE, regTemp, regDst, finalOffset); @@ -9566,7 +9569,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) loadIns = ins_Load(TYP_UBYTE); // INS_ldrb storeIns = ins_Store(TYP_UBYTE); // INS_strb - if (oper == GT_COPYBLK) + if (isCopyBlk) { getEmitter()->emitIns_R_R_I(loadIns, EA_1BYTE, regTemp, regSrc, finalOffset); getEmitter()->emitIns_R_R_I(storeIns, EA_1BYTE, regTemp, regDst, finalOffset); @@ -9613,7 +9616,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) regSet.rsLockUsedReg(RBM_ARG_0 | RBM_ARG_1 | RBM_ARG_2); - genEmitHelperCall(oper == GT_COPYBLK ? CORINFO_HELP_MEMCPY + genEmitHelperCall(isCopyBlk ? CORINFO_HELP_MEMCPY /* GT_INITBLK */ : CORINFO_HELP_MEMSET, 0, EA_UNKNOWN); @@ -9626,7 +9629,7 @@ void CodeGen::genCodeForBlkOp(GenTreePtr tree, regMaskTP destReg) genReleaseReg(opsPtr[2]); } - if ((oper == GT_COPYBLK) && tree->AsBlkOp()->IsVolatile()) + if (isCopyBlk && tree->AsBlkOp()->IsVolatile()) { // Emit a memory barrier instruction after the CopyBlk instGen_MemoryBarrier(); diff --git a/src/coreclr/src/jit/codegenxarch.cpp b/src/coreclr/src/jit/codegenxarch.cpp index 1be6bb7..b5f85aa 100755 --- a/src/coreclr/src/jit/codegenxarch.cpp +++ b/src/coreclr/src/jit/codegenxarch.cpp @@ -2176,6 +2176,17 @@ void CodeGen::genCodeForTreeNode(GenTreePtr treeNode) break; } #endif // !defined(_TARGET_64BIT_) + +#ifdef FEATURE_SIMD + if (varTypeIsSIMD(targetType) && (targetReg != REG_NA) && op1->IsCnsIntOrI()) + { + // This is only possible for a zero-init. + noway_assert(op1->IsIntegralConst(0)); + genSIMDZero(targetType, varDsc->lvBaseType, targetReg); + genProduceReg(treeNode); + break; + } +#endif // FEATURE_SIMD genConsumeRegs(op1); @@ -2183,7 +2194,7 @@ void CodeGen::genCodeForTreeNode(GenTreePtr treeNode) { // stack store emit->emitInsMov(ins_Store(targetType, compiler->isSIMDTypeLocalAligned(lclNum)), - emitTypeSize(treeNode), treeNode); + emitTypeSize(targetType), treeNode); varDsc->lvRegNum = REG_STK; } else diff --git a/src/coreclr/src/jit/earlyprop.cpp b/src/coreclr/src/jit/earlyprop.cpp index a4a9ea5..70d1012 100644 --- a/src/coreclr/src/jit/earlyprop.cpp +++ b/src/coreclr/src/jit/earlyprop.cpp @@ -238,8 +238,12 @@ bool Compiler::optEarlyPropRewriteTree(GenTreePtr tree) objectRefPtr = tree->gtOp.gtOp1; propKind = optPropKind::OPK_ARRAYLEN; } - else if (tree->OperGet() == GT_IND) + else if ((tree->OperGet() == GT_IND) && !varTypeIsStruct(tree)) { + // TODO-1stClassStructs: The above condition should apply equally to all indirections, + // but previously the implicit indirections due to a struct assignment were not + // considered, so we are currently limiting it to non-structs to preserve existing + // behavior. // optFoldNullCheck takes care of updating statement info if a null check is removed. optFoldNullCheck(tree); diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index 4881a639ee..4892d1c 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -1080,6 +1080,17 @@ public: bool OperIsInitBlkOp() const; bool OperIsDynBlkOp(); + static + bool OperIsBlk(genTreeOps gtOper) + { + return (gtOper == GT_OBJ); + } + + bool OperIsBlk() const + { + return OperIsBlk(OperGet()); + } + bool OperIsPutArgStk() const { return gtOper == GT_PUTARG_STK; @@ -1492,6 +1503,19 @@ public: void ChangeOper(genTreeOps oper, ValueNumberUpdate vnUpdate = CLEAR_VN); void ChangeOperUnchecked(genTreeOps oper); + void ChangeType(var_types newType) + { + var_types oldType = gtType; + gtType = newType; + GenTree* node = this; + while (node->gtOper == GT_COMMA) + { + node = node->gtGetOp2(); + assert(node->gtType == oldType); + node->gtType = newType; + } + } + bool IsLocal() const { return OperIsLocal(OperGet()); diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 6157611..fd3bda7 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -607,12 +607,16 @@ inline void Compiler::impAppendStmt(GenTreePtr stmt, unsigned chkLevel) GenTreePtr expr = stmt->gtStmt.gtStmtExpr; unsigned flags = expr->gtFlags & GTF_GLOB_EFFECT; - /* Assignment to (unaliased) locals don't count as a side-effect as - we handle them specially using impSpillLclRefs(). Temp locals should - be fine too. */ + // Assignment to (unaliased) locals don't count as a side-effect as + // we handle them specially using impSpillLclRefs(). Temp locals should + // be fine too. + // TODO-1stClassStructs: The check below should apply equally to struct assignments, + // but previously the block ops were always being marked GTF_GLOB_REF, even if + // the operands could not be global refs. if ((expr->gtOper == GT_ASG) && (expr->gtOp.gtOp1->gtOper == GT_LCL_VAR) && - !(expr->gtOp.gtOp1->gtFlags & GTF_GLOB_REF) && !gtHasLocalsWithAddrOp(expr->gtOp.gtOp2)) + !(expr->gtOp.gtOp1->gtFlags & GTF_GLOB_REF) && !gtHasLocalsWithAddrOp(expr->gtOp.gtOp2) && + !varTypeIsStruct(expr->gtOp.gtOp1)) { unsigned op2Flags = expr->gtOp.gtOp2->gtFlags & GTF_GLOB_EFFECT; assert(flags == (op2Flags | GTF_ASG)); @@ -1086,7 +1090,7 @@ GenTreePtr Compiler::impAssignStruct(GenTreePtr dest, GenTreePtr destAddr; - if (dest->gtOper == GT_IND || dest->gtOper == GT_OBJ) + if (dest->gtOper == GT_IND || dest->OperIsBlk()) { destAddr = dest->gtOp.gtOp1; } @@ -1555,7 +1559,7 @@ GenTreePtr Compiler::impNormStructVal(GenTreePtr structVal, } else { - // In general a OBJ is an IND and could raise an exception + // In general a OBJ is an indirection and could raise an exception. structObj->gtFlags |= GTF_EXCEPT; } return (structObj); @@ -3070,14 +3074,15 @@ GenTreePtr Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig) // // At this point we are ready to commit to implementing the InitializeArray - // instrinsic using a struct assignment. Pop the arguments from the stack and + // intrinsic using a struct assignment. Pop the arguments from the stack and // return the struct assignment node. // impPopStack(); impPopStack(); - GenTreePtr dst; + const unsigned blkSize = size.Value(); + GenTreePtr dst; if (isMDArray) { @@ -3090,10 +3095,11 @@ GenTreePtr Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig) dst = gtNewOperNode(GT_ADDR, TYP_BYREF, gtNewIndexRef(elementType, arrayLocalNode, gtNewIconNode(0))); } + GenTreePtr srcAddr = gtNewIconHandleNode((size_t) initData, GTF_ICON_STATIC_HDL); return gtNewBlkOpNode(GT_COPYBLK, - dst, // dst - gtNewIconHandleNode((size_t)initData, GTF_ICON_STATIC_HDL), // src - gtNewIconNode(size.Value()), // size + dst, // dst + srcAddr, // src + gtNewIconNode(blkSize), // size false); } @@ -13073,7 +13079,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) assert(!"Unexpected fieldAccessor"); } - /* Create the member assignment, unless we have a struct */ + // Create the member assignment, unless we have a struct. + // TODO-1stClassStructs: This could be limited to TYP_STRUCT, to avoid extra copies. bool deferStructAssign = varTypeIsStruct(lclTyp); if (!deferStructAssign) diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index c4ae0c8..5c57a88 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -3319,14 +3319,15 @@ void Lowering::LowerAdd(GenTreePtr* pTree, Compiler::fgWalkData* data) return; } - // if this is a child of an indir, let the parent handle it - if (data->parentStack->Index(1)->OperIsIndir()) + // If this is a child of an indir, and it is not a block op, let the parent handle it. + GenTree* parent = data->parentStack->Index(1); + if (parent->OperIsIndir() && !varTypeIsStruct(parent)) { return; } // if there is a chain of adds, only look at the topmost one - if (data->parentStack->Index(1)->gtOper == GT_ADD) + if (parent->gtOper == GT_ADD) { return; } diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 277c375..154b8e2 100755 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -2030,6 +2030,23 @@ GenTreePtr Compiler::fgMakeTmpArgNode( arg->gtFlags |= GTF_DONT_CSE; +#else // !FEATURE_UNIX_AMD64_STRUCT_PASSING + // Can this type be passed in a single register? + // If so, the following call will return the corresponding primitive type. + // Otherwise, it will return TYP_UNKNOWN and we will pass by reference. + + bool passedInRegisters = false; + structPassingKind kind; + CORINFO_CLASS_HANDLE clsHnd = varDsc->lvVerTypeInfo.GetClassHandle(); + var_types structBaseType = getPrimitiveTypeForStruct(lvaLclExactSize(tmpVarNum), clsHnd); + + if (structBaseType != TYP_UNKNOWN) + { + passedInRegisters = true; + type = structBaseType; + } +#endif // !FEATURE_UNIX_AMD64_STRUCT_PASSING + // If it is passed in registers, don't get the address of the var. Make it a // field instead. It will be loaded in registers with putarg_reg tree in lower. if (passedInRegisters) @@ -2041,61 +2058,7 @@ GenTreePtr Compiler::fgMakeTmpArgNode( { arg = gtNewOperNode(GT_ADDR, type, arg); addrNode = arg; - } - -#else // !FEATURE_UNIX_AMD64_STRUCT_PASSING - - unsigned structSize = lvaLclExactSize(tmpVarNum); - switch (structSize) - { - case 1: - type = TYP_BYTE; - break; - case 2: - type = TYP_SHORT; - break; -#if defined(_TARGET_AMD64_) - case 4: - type = TYP_INT; - break; -#elif defined(_TARGET_ARM64_) - case 3: - case 4: - type = TYP_INT; - break; - case 5: - case 6: - case 7: - type = TYP_I_IMPL; - break; -#endif // defined (_TARGET_ARM64_) - case 8: - switch (*lvaGetGcLayout(tmpVarNum)) - { - case TYPE_GC_NONE: - type = TYP_I_IMPL; - break; - case TYPE_GC_REF: - type = TYP_REF; - break; - case TYPE_GC_BYREF: - type = TYP_BYREF; - break; - default: - unreached(); - } - break; - default: - break; - } - - // If we didn't change the type of the struct, it means - // its structure doesn't support to be passed directly through a - // register, so we need to pass a pointer to the destination where - // where we copied the struct to. - if (type == varDsc->TypeGet()) - { #if FEATURE_MULTIREG_ARGS #ifdef _TARGET_ARM64_ assert(varTypeIsStruct(type)); @@ -2103,40 +2066,27 @@ GenTreePtr Compiler::fgMakeTmpArgNode( { // ToDo-ARM64: Consider using: arg->ChangeOper(GT_LCL_FLD); // as that is how FEATURE_UNIX_AMD64_STRUCT_PASSING works. - // Create a GT_OBJ for the argument - // This will be passed by value in two registers - arg = gtNewOperNode(GT_ADDR, TYP_BYREF, arg); - addrNode = arg; + // We will create a GT_OBJ for the argument below. + // This will be passed by value in two registers. + assert(addrNode != nullptr); // Create an Obj of the temp to use it as a call argument. arg = gtNewObjNode(lvaGetStruct(tmpVarNum), arg); } - else #endif // _TARGET_ARM64_ #endif // FEATURE_MULTIREG_ARGS - { - arg = gtNewOperNode(GT_ADDR, TYP_I_IMPL, arg); - addrNode = arg; - } - } - else // type was changed from a struct to a scalar type - { - arg->ChangeOper(GT_LCL_FLD); - arg->gtType = type; } -#endif // !FEATURE_UNIX_AMD64_STRUCT_PASSING #else // not (_TARGET_AMD64_ or _TARGET_ARM64_) // other targets, we pass the struct by value assert(varTypeIsStruct(type)); - arg = gtNewOperNode(GT_ADDR, TYP_BYREF, arg); - addrNode = arg; + addrNode = gtNewOperNode(GT_ADDR, TYP_BYREF, arg); // Get a new Obj node temp to use it as a call argument. // gtNewObjNode will set the GTF_EXCEPT flag if this is not a local stack object. - arg = gtNewObjNode(lvaGetStruct(tmpVarNum), arg); + arg = gtNewObjNode(lvaGetStruct(tmpVarNum), addrNode); #endif // not (_TARGET_AMD64_ or _TARGET_ARM64_) @@ -3441,6 +3391,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode) argObj = argObj->gtOp.gtOp2; } + // TODO-1stClassStructs: An OBJ node should not be required for lclVars. if (argObj->gtOper != GT_OBJ) { BADCODE("illegal argument tree in fgMorphArgs"); @@ -5121,6 +5072,9 @@ void Compiler::fgMakeOutgoingStructArgCopy( GenTreePtr dest = gtNewLclvNode(tmp, lvaTable[tmp].lvType); dest->gtFlags |= (GTF_DONT_CSE | GTF_VAR_DEF); // This is a def of the local, "entire" by construction. dest = gtNewOperNode(GT_ADDR, TYP_BYREF, dest); + + // TODO-Cleanup: This probably shouldn't be done here because arg morphing is done prior + // to ref counting of the lclVars. lvaTable[tmp].incRefCnts(compCurBB->getBBWeight(this), this); GenTreePtr src; @@ -8292,21 +8246,26 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree) { genTreeOps oper = tree->gtOper; - // Only xxBlk opcodes are possible + // This must be a block assignment. noway_assert(tree->OperIsBlkOp()); - - GenTreePtr dest = tree->gtOp.gtOp1->gtOp.gtOp1; // Dest address - GenTreePtr src = tree->gtOp.gtOp1->gtOp.gtOp2; // Src - GenTreePtr blkShape = tree->gtOp.gtOp2; // [size/clsHnd] - bool volatil = tree->AsBlkOp()->IsVolatile(); + var_types asgType = TYP_STRUCT; + + GenTreePtr dest = tree->gtOp.gtOp1->gtOp.gtOp1; // Dest address + GenTreePtr src = tree->gtOp.gtOp1->gtOp.gtOp2; // Src + GenTreePtr blkShape = tree->gtOp.gtOp2; // [size/clsHnd] + bool volatil = tree->AsBlkOp()->IsVolatile(); + unsigned destVarNum = BAD_VAR_NUM; + LclVarDsc* destVarDsc = nullptr; GenTreePtr result; - GenTreePtr lclVarTree; + GenTreePtr lclVarTree = nullptr; + bool isCopyBlock = tree->OperIsCopyBlkOp(); + bool isInitBlock = !isCopyBlock; // The dest must be an address noway_assert(genActualType(dest->gtType) == TYP_I_IMPL || dest->gtType == TYP_BYREF); // For COPYBLK the src must be an address - noway_assert(!tree->OperIsCopyBlkOp() || (genActualType(src->gtType) == TYP_I_IMPL || src->gtType == TYP_BYREF)); + noway_assert(!isCopyBlock || (genActualType( src->gtType) == TYP_I_IMPL || src->gtType == TYP_BYREF)); // For INITBLK the src must be a TYP_INT noway_assert(oper != GT_INITBLK || (genActualType(src->gtType) == TYP_INT)); @@ -8316,11 +8275,10 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree) CORINFO_CLASS_HANDLE clsHnd; size_t size; - var_types type = TYP_UNDEF; if (blkShape->gtOper != GT_CNS_INT) { - goto GENERAL_BLKOP; + return nullptr; } #ifdef FEATURE_SIMD @@ -8332,28 +8290,18 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree) // routine will prevent rationalizer logic and we might end up with // GT_ADDR(GT_SIMD) node post rationalization, leading to a noway assert // in codegen. + // TODO-1stClassStructs: This is here to preserve old behavior. + // It should be eliminated. if (src->OperGet() == GT_ADDR && src->gtGetOp1()->OperGet() == GT_SIMD) { - goto GENERAL_BLKOP; + return nullptr; } #endif if (!blkShape->IsIconHandle()) { - clsHnd = nullptr; + clsHnd = NO_CLASS_HANDLE; size = blkShape->gtIntCon.gtIconVal; - - /* A four byte BLK_COPY can be treated as an integer asignment */ - if (size == 4) - { - type = TYP_INT; - } -#ifdef _TARGET_64BIT_ - if (size == 8) - { - type = TYP_LONG; - } -#endif } else { @@ -8365,13 +8313,6 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree) // The EE currently does not allow this, but we may change. Lets assert it // just to be safe. noway_assert(info.compCompHnd->getClassSize(clsHnd) == size); - - if (size == REGSIZE_BYTES) - { - BYTE gcPtr; - info.compCompHnd->getClassGClayout(clsHnd, &gcPtr); - type = getJitGCType(gcPtr); - } } // @@ -8384,115 +8325,138 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree) // [dest] [src] // - switch (size) + if (size == REGSIZE_BYTES) { - case 1: - type = TYP_BYTE; - goto ONE_SIMPLE_ASG; - case 2: - type = TYP_SHORT; - goto ONE_SIMPLE_ASG; + if (clsHnd == NO_CLASS_HANDLE) + { + // A register-sized cpblk can be treated as an integer asignment. + asgType = TYP_I_IMPL; + } + else + { + BYTE gcPtr; + info.compCompHnd->getClassGClayout(clsHnd, &gcPtr); + asgType = getJitGCType(gcPtr); + } + } + else + { + switch (size) + { + case 1: + asgType = TYP_BYTE; + break; + case 2: + asgType = TYP_SHORT; + break; #ifdef _TARGET_64BIT_ - case 4: - type = TYP_INT; - goto ONE_SIMPLE_ASG; + case 4: + asgType = TYP_INT; + break; #endif // _TARGET_64BIT_ + } + } - case REGSIZE_BYTES: - noway_assert(type != TYP_UNDEF); - - ONE_SIMPLE_ASG: - - noway_assert(size <= REGSIZE_BYTES); - - // For INITBLK, a non constant source is not going to allow us to fiddle - // with the bits to create a single assigment. + // TODO-1stClassStructs: Change this to asgType != TYP_STRUCT. + if (!varTypeIsStruct(asgType)) + { + // For initBlk, a non constant source is not going to allow us to fiddle + // with the bits to create a single assigment. + noway_assert(size <= REGSIZE_BYTES); - if ((oper == GT_INITBLK) && (src->gtOper != GT_CNS_INT)) - { - goto GENERAL_BLKOP; - } + if (isInitBlock && (src->gtOper != GT_CNS_INT)) + { + return nullptr; + } - if (impIsAddressInLocal(dest, &lclVarTree)) - { + bool needsIndirection = true; + if (impIsAddressInLocal(dest, &lclVarTree)) + { + destVarNum = lclVarTree->AsLclVarCommon()->gtLclNum; + destVarDsc = &(lvaTable[destVarNum]); + } + if (destVarDsc != nullptr) + { #if LOCAL_ASSERTION_PROP - // Kill everything about dest - if (optLocalAssertionProp) + // Kill everything about dest + if (optLocalAssertionProp) + { + if (optAssertionCount > 0) { - if (optAssertionCount > 0) - { - fgKillDependentAssertions(lclVarTree->gtLclVarCommon.gtLclNum DEBUGARG(tree)); - } + fgKillDependentAssertions(destVarNum DEBUGARG(tree)); } + } #endif // LOCAL_ASSERTION_PROP - unsigned lclNum = lclVarTree->gtLclVarCommon.gtLclNum; - // A previous incarnation of this code also required the local not to be - // address-exposed(=taken). That seems orthogonal to the decision of whether - // to do field-wise assignments: being address-exposed will cause it to be - // "dependently" promoted, so it will be in the right memory location. One possible - // further reason for avoiding field-wise stores is that the struct might have alignment-induced - // holes, whose contents could be meaningful in unsafe code. If we decide that's a valid - // concern, then we could compromise, and say that address-exposed + fields do not completely cover the - // memory of the struct prevent field-wise assignments. Same situation exists for the "src" decision. - if (varTypeIsStruct(lclVarTree) && (lvaTable[lclNum].lvPromoted || lclVarIsSIMDType(lclNum))) - { - - // Let fgMorphInitBlock handle it. (Since we'll need to do field-var-wise assignments.) - goto GENERAL_BLKOP; - } - else if (!varTypeIsFloating(lclVarTree->TypeGet()) && - size == genTypeSize(var_types(lvaTable[lclNum].lvType))) - { - // Use the dest local var directly. - dest = lclVarTree; - type = lvaTable[lclNum].lvType; // Make the type used in the GT_IND node match - - // If the block operation had been a write to a local var of a small int type, - // of the exact size of the small int type, and the var is NormalizeOnStore, - // we would have labeled it GTF_VAR_USEASG, because the block operation wouldn't - // have done that normalization. If we're now making it into an assignment, - // the NormalizeOnStore will work, and it can be a full def. - if (lvaTable[lclNum].lvNormalizeOnStore()) - { - dest->gtFlags &= (~GTF_VAR_USEASG); - } + // A previous incarnation of this code also required the local not to be + // address-exposed(=taken). That seems orthogonal to the decision of whether + // to do field-wise assignments: being address-exposed will cause it to be + // "dependently" promoted, so it will be in the right memory location. One possible + // further reason for avoiding field-wise stores is that the struct might have alignment-induced + // holes, whose contents could be meaningful in unsafe code. If we decide that's a valid + // concern, then we could compromise, and say that address-exposed + fields do not completely cover the + // memory of the struct prevent field-wise assignments. Same situation exists for the "src" decision. + if (varTypeIsStruct(lclVarTree) && (destVarDsc->lvPromoted || destVarDsc->lvIsSIMDType())) + { + // Let fgMorphInitBlock handle it. (Since we'll need to do field-var-wise assignments.) + return nullptr; + } + else if (!varTypeIsFloating(lclVarTree->TypeGet()) && + size == genTypeSize(var_types(lvaTable[destVarNum].lvType))) + { + // Use the dest local var directly. + dest = lclVarTree; + asgType = destVarDsc->lvType; // Make the type used in the GT_IND node match + needsIndirection = false; - goto _DoneDest; - } - else + // If the block operation had been a write to a local var of a small int type, + // of the exact size of the small int type, and the var is NormalizeOnStore, + // we would have labeled it GTF_VAR_USEASG, because the block operation wouldn't + // have done that normalization. If we're now making it into an assignment, + // the NormalizeOnStore will work, and it can be a full def. + if (destVarDsc->lvNormalizeOnStore()) { - // Could be a non-promoted struct, or a floating point type local, or - // an int subject to a partial write. Don't enregister. - lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_LocalField)); - // Fall through to indirect the dest node. + dest->gtFlags &= (~GTF_VAR_USEASG); } + } + else + { + // Could be a non-promoted struct, or a floating point type local, or + // an int subject to a partial write. Don't enregister. + lvaSetVarDoNotEnregister(destVarNum DEBUGARG(DNER_LocalField)); + // Mark the local var tree as a definition point of the local. lclVarTree->gtFlags |= GTF_VAR_DEF; - if (size < lvaTable[lclNum].lvExactSize) + if (size < destVarDsc->lvExactSize) { // If it's not a full-width assignment.... lclVarTree->gtFlags |= GTF_VAR_USEASG; } } + } + if (needsIndirection) + { // Check to ensure we are not creating a reducible *(& ... ) if (dest->gtOper == GT_ADDR) { GenTreePtr addrOp = dest->gtOp.gtOp1; // Ignore reinterpret casts between int/gc - if ((addrOp->TypeGet() == type) || + if ((addrOp->TypeGet() == asgType) || (varTypeIsIntegralOrI(addrOp) && (genTypeSize(addrOp->TypeGet()) == size))) { dest = addrOp; - type = addrOp->TypeGet(); - goto _DoneDest; + asgType = addrOp->TypeGet(); + needsIndirection = false; } } + } + if (needsIndirection) + { // Indirect the dest node. - dest = gtNewOperNode(GT_IND, type, dest); + dest = gtNewOperNode(GT_IND, asgType, dest); // If we have no information about the destination, we have to assume it could // live anywhere (not just in the GC heap). @@ -8503,56 +8467,55 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree) { dest->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE); } + } - _DoneDest:; - - if (volatil) - { - dest->gtFlags |= GTF_DONT_CSE; - } + if (volatil) + { + dest->gtFlags |= GTF_DONT_CSE; + } - if (tree->OperIsCopyBlkOp()) + LclVarDsc* srcVarDsc = nullptr; + if (isCopyBlock) + { + if (impIsAddressInLocal(src, &lclVarTree)) { - if (impIsAddressInLocal(src, &lclVarTree)) + srcVarDsc = &(lvaTable[lclVarTree->AsLclVarCommon()->gtLclNum]); + if (varTypeIsStruct(lclVarTree) && (srcVarDsc->lvPromoted || srcVarDsc->lvIsSIMDType())) + { + // Let fgMorphCopyBlock handle it. + return nullptr; + } + else if (!varTypeIsFloating(lclVarTree->TypeGet()) && + size == genTypeSize(genActualType(lclVarTree->TypeGet()))) + { + // Use the src local var directly. + src = lclVarTree; + } + else { - unsigned lclNum = lclVarTree->gtLclVarCommon.gtLclNum; - if (varTypeIsStruct(lclVarTree) && (lvaTable[lclNum].lvPromoted || lclVarIsSIMDType(lclNum))) - { - // Let fgMorphCopyBlock handle it. - goto GENERAL_BLKOP; - } - else if (!varTypeIsFloating(lclVarTree->TypeGet()) && - size == genTypeSize(genActualType(lclVarTree->TypeGet()))) - { - /* Use the src local var directly */ - src = lclVarTree; - goto _DoneSrc; - } - else - { #ifndef LEGACY_BACKEND - // The source argument of the copyblk can potentially - // be accessed only through indir(addr(lclVar)) - // or indir(lclVarAddr) in rational form and liveness - // won't account for these uses. That said, - // we have to mark this local as address exposed so - // we don't delete it as a dead store later on. - unsigned lclVarNum = lclVarTree->gtLclVarCommon.gtLclNum; - lvaTable[lclVarNum].lvAddrExposed = true; - lvaSetVarDoNotEnregister(lclVarNum DEBUGARG(DNER_AddrExposed)); + // The source argument of the copyblk can potentially + // be accessed only through indir(addr(lclVar)) + // or indir(lclVarAddr) in rational form and liveness + // won't account for these uses. That said, + // we have to mark this local as address exposed so + // we don't delete it as a dead store later on. + unsigned lclVarNum = lclVarTree->gtLclVarCommon.gtLclNum; + lvaTable[lclVarNum].lvAddrExposed = true; + lvaSetVarDoNotEnregister(lclVarNum DEBUGARG(DNER_AddrExposed)); #else // LEGACY_BACKEND - lvaSetVarDoNotEnregister(lclVarTree->gtLclVarCommon.gtLclNum DEBUGARG(DNER_LocalField)); + lvaSetVarDoNotEnregister(lclVarTree->gtLclVarCommon.gtLclNum DEBUGARG(DNER_LocalField)); #endif // LEGACY_BACKEND - - // Fall through to indirect the src node. - } } + } + if (src != lclVarTree) + { // Indirect the src node. - src = gtNewOperNode(GT_IND, type, src); + src = gtNewOperNode(GT_IND, asgType, src); // If we have no information about the src, we have to assume it could // live anywhere (not just in the GC heap). @@ -8563,31 +8526,28 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree) { src->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE); } - - _DoneSrc:; - - if (volatil) - { - src->gtFlags |= GTF_DONT_CSE; - } } - else // (oper == GT_INITBLK) + + if (volatil) { - // This will mutate the integer constant, in place, to be the correct - // value for the type were are using in the assignment. - src->AsIntCon()->FixupInitBlkValue(type); + src->gtFlags |= GTF_DONT_CSE; } + } + else // (oper == GT_INITBLK) + { + // This will mutate the integer constant, in place, to be the correct + // value for the type we are using in the assignment. + src->AsIntCon()->FixupInitBlkValue(asgType); + } - /* Create the assignment node */ + // Create the assignment node. - result = gtNewAssignNode(dest, src); - result->gtType = type; + result = gtNewAssignNode(dest, src); + result->gtType = asgType; - return result; + return result; } -GENERAL_BLKOP: - return nullptr; } @@ -16834,20 +16794,10 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTreePtr* pTree, fgWalkData* fgWalkPre } else { - // Change X into *X - // First, save the original type, then change the tree type to TYP_BYREF (otherwise we - // will get an assert when we try to clone the lclVar node because the lclVar is now TYP_BYREF - // and the types have to match). The reason we clone the lclVar is that we don't pass a - // possible-modified tree back to the caller, so we modify the original lclVar node in-place - // to the GT_IND. + // Change X into *X. var_types structType = tree->gtType; - lclVarTree = gtClone(tree); - // Now, set the types appropriately. - lclVarTree->gtType = TYP_BYREF; - tree->gtType = structType; - // Now, "insert" the GT_IND by changing the oper of the original node and setting its op1. - tree->SetOper(GT_IND); - tree->gtOp.gtOp1 = lclVarTree; + tree->gtType = TYP_BYREF; + tree = gtNewOperNode(GT_IND, structType, tree); // TODO-CQ: If the VM ever stops violating the ABI and passing heap references // we could remove TGTANYWHERE tree->gtFlags = ((tree->gtFlags & GTF_COMMON_MASK) | GTF_IND_TGTANYWHERE); @@ -17304,6 +17254,10 @@ bool Compiler::fgFitsInOrNotLoc(GenTreePtr tree, unsigned width) CORINFO_CLASS_HANDLE fldClass = info.compCompHnd->getFieldClass(tree->gtField.gtFldHnd); return width <= info.compCompHnd->getClassSize(fldClass); } + else if (tree->OperGet() == GT_INDEX) + { + return width <= tree->gtIndex.gtIndElemSize; + } else { return false; diff --git a/src/coreclr/src/jit/rationalize.cpp b/src/coreclr/src/jit/rationalize.cpp index 293a1d7..6856ff9 100644 --- a/src/coreclr/src/jit/rationalize.cpp +++ b/src/coreclr/src/jit/rationalize.cpp @@ -1568,6 +1568,7 @@ Compiler::fgWalkResult Rationalizer::SimpleTransformHelper(GenTree** ppTree, Com case GT_LCL_FLD: case GT_STORE_LCL_FLD: + // TODO-1stClassStructs: Eliminate this. FixupIfSIMDLocal(comp, tree->AsLclVarCommon()); break; @@ -1590,7 +1591,9 @@ Compiler::fgWalkResult Rationalizer::SimpleTransformHelper(GenTree** ppTree, Com GenTreeSIMD* simdTree = (*ppTree)->AsSIMD(); simdSize = simdTree->gtSIMDSize; var_types simdType = comp->getSIMDTypeForSize(simdSize); - // TODO-Cleanup: This is no-longer required once we plumb SIMD types thru front-end + // TODO-1stClassStructs: This should be handled more generally for enregistered or promoted + // structs that are passed or returned in a different register type than their enregistered + // type(s). if (simdTree->gtType == TYP_I_IMPL && simdTree->gtSIMDSize == TARGET_POINTER_SIZE) { // This happens when it is consumed by a GT_RET_EXPR. @@ -1676,8 +1679,9 @@ Compiler::fgWalkResult Rationalizer::SimpleTransformHelper(GenTree** ppTree, Com // Return Value: // None. // -// TODO-Cleanup: Once SIMD types are plumbed through the frontend, this will no longer -// be required. +// TODO-1stClassStructs: This is now only here to preserve existing behavior. It is actually not +// desirable to change the lclFld nodes back to TYP_SIMD (it will cause them to be loaded +// into a vector register, and then moved to an int register). void Rationalizer::FixupIfSIMDLocal(Compiler* comp, GenTreeLclVarCommon* tree) { diff --git a/src/coreclr/src/jit/ssabuilder.cpp b/src/coreclr/src/jit/ssabuilder.cpp index 3cdf659..2da69024 100644 --- a/src/coreclr/src/jit/ssabuilder.cpp +++ b/src/coreclr/src/jit/ssabuilder.cpp @@ -917,7 +917,7 @@ void SsaBuilder::TreeRenameVariables(GenTree* tree, BasicBlock* block, SsaRename { GenTreePtr lhs = tree->gtOp.gtOp1->gtEffectiveVal(/*commaOnly*/ true); GenTreePtr trueLhs = lhs->gtEffectiveVal(/*commaOnly*/ true); - if (trueLhs->OperGet() == GT_IND) + if (trueLhs->OperIsIndir()) { trueLhs->gtFlags |= GTF_IND_ASG_LHS; } diff --git a/src/coreclr/src/jit/valuenum.cpp b/src/coreclr/src/jit/valuenum.cpp index fcf4702..7994254 100644 --- a/src/coreclr/src/jit/valuenum.cpp +++ b/src/coreclr/src/jit/valuenum.cpp @@ -2766,7 +2766,7 @@ ValueNum Compiler::fgValueNumberArrIndexVal(GenTreePtr tree, ValueNum excVN, FieldSeqNode* fldSeq) { - assert(tree == nullptr || tree->OperGet() == GT_IND); + assert(tree == nullptr || tree->OperIsIndir()); // The VN inputs are required to be non-exceptional values. assert(arrVN == vnStore->VNNormVal(arrVN)); @@ -4773,12 +4773,15 @@ void Compiler::fgValueNumberTreeConst(GenTreePtr tree) void Compiler::fgValueNumberBlockAssignment(GenTreePtr tree, bool evalAsgLhsInd) { + GenTree* lhsAddr = tree->gtGetOp1()->gtGetOp1(); + GenTree* rhsAddrOrVal = tree->gtGetOp1()->gtGetOp2(); + #ifdef DEBUG // Sometimes we query the heap ssa map, and need a dummy location for the ignored result. unsigned heapSsaNum; #endif - if (tree->OperGet() == GT_INITBLK) + if (tree->OperIsInitBlkOp()) { GenTreeLclVarCommon* lclVarTree; bool isEntire; @@ -4798,7 +4801,7 @@ void Compiler::fgValueNumberBlockAssignment(GenTreePtr tree, bool evalAsgLhsInd) unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); ValueNum initBlkVN = ValueNumStore::NoVN; - GenTreePtr initConst = tree->gtGetOp1()->gtGetOp2(); + GenTreePtr initConst = rhsAddrOrVal; if (isEntire && initConst->OperGet() == GT_CNS_INT) { unsigned initVal = 0xFF & (unsigned)initConst->AsIntConCommon()->IconValue(); @@ -4851,16 +4854,15 @@ void Compiler::fgValueNumberBlockAssignment(GenTreePtr tree, bool evalAsgLhsInd) // Should not have been recorded as updating the heap. assert(!GetHeapSsaMap()->Lookup(tree, &heapSsaNum)); - unsigned lhsLclNum = lclVarTree->GetLclNum(); - LclVarDsc* rhsVarDsc = &lvaTable[lhsLclNum]; + unsigned lhsLclNum = lclVarTree->GetLclNum(); // If it's excluded from SSA, don't need to do anything. if (!fgExcludeFromSsa(lhsLclNum)) { unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); // For addr-of-local expressions, lib/cons shouldn't matter. - assert(tree->gtOp.gtOp1->gtOp.gtOp1->gtVNPair.BothEqual()); - ValueNum lhsAddrVN = tree->gtOp.gtOp1->gtOp.gtOp1->GetVN(VNK_Liberal); + assert(lhsAddr->gtVNPair.BothEqual()); + ValueNum lhsAddrVN = lhsAddr->GetVN(VNK_Liberal); // Unpack the PtrToLoc value number of the address. assert(vnStore->IsVNFunc(lhsAddrVN)); @@ -4872,7 +4874,7 @@ void Compiler::fgValueNumberBlockAssignment(GenTreePtr tree, bool evalAsgLhsInd) FieldSeqNode* lhsFldSeq = vnStore->FieldSeqVNToFieldSeq(lhsAddrFuncApp.m_args[1]); // Now we need to get the proper RHS. - GenTreePtr srcAddr = tree->gtOp.gtOp1->gtOp.gtOp2; + GenTreePtr srcAddr = rhsAddrOrVal; VNFuncApp srcAddrFuncApp; GenTreeLclVarCommon* rhsLclVarTree = nullptr; FieldSeqNode* rhsFldSeq = nullptr; @@ -5022,7 +5024,12 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) var_types typ = tree->TypeGet(); if (GenTree::OperIsConst(oper)) { - fgValueNumberTreeConst(tree); + // If this is a struct assignment, with a constant rhs, it is an initBlk, and it is not + // really useful to value number the constant. + if (!varTypeIsStruct(tree)) + { + fgValueNumberTreeConst(tree); + } } else if (GenTree::OperIsLeaf(oper)) { @@ -5308,8 +5315,8 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) // But we didn't know that the parent was an op=. We do now, so go back and evaluate it. // (We actually check if the effective val is the IND. We will have evaluated any non-last // args of an LHS comma already -- including their heap effects.) - GenTreePtr lhsVal = lhs->gtEffectiveVal(/*commaOnly*/ true); - if ((lhsVal->OperGet() == GT_IND) || (lhsVal->OperGet() == GT_CLS_VAR)) + GenTreePtr lhsVal = lhs->gtEffectiveVal(/*commaOnly*/true); + if (lhsVal->OperIsIndir() || (lhsVal->OperGet() == GT_CLS_VAR)) { fgValueNumberTree(lhsVal, /*evalAsgLhsInd*/ true); } @@ -5924,11 +5931,12 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) { // They just cancel, so fetch the ValueNumber from the op1 of the GT_IND node. // - tree->gtVNPair = arg->gtOp.gtOp1->gtVNPair; + GenTree* addr = arg->AsIndir()->Addr(); + tree->gtVNPair = addr->gtVNPair; - // For the CSE phase mark the GT_IND child as GTF_DONT_CSE + // For the CSE phase mark the address as GTF_DONT_CSE // because it will end up with the same value number as tree (the GT_ADDR). - arg->gtOp.gtOp1->gtFlags |= GTF_DONT_CSE; + addr->gtFlags |= GTF_DONT_CSE; } } else @@ -5941,7 +5949,7 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) { // So far, we handle cases in which the address is a ptr-to-local, or if it's // a pointer to an object field. - GenTreePtr addr = tree->gtOp.gtOp1; + GenTreePtr addr = tree->AsIndir()->Addr(); GenTreeLclVarCommon* lclVarTree = nullptr; FieldSeqNode* fldSeq1 = nullptr; FieldSeqNode* fldSeq2 = nullptr; @@ -6294,20 +6302,20 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) ValueNumPair op2vnp; ValueNumPair op2Xvnp = ValueNumStore::VNPForEmptyExcSet(); - if ((tree->gtOp.gtOp2->OperGet() == GT_IND) && (tree->gtOp.gtOp2->gtFlags & GTF_IND_ASG_LHS)) + GenTree* op2 = tree->gtGetOp2(); + if (op2->OperIsIndir() && ((op2->gtFlags & GTF_IND_ASG_LHS) != 0)) { // If op2 represents the lhs of an assignment then we give a VNForVoid for the lhs op2vnp = ValueNumPair(ValueNumStore::VNForVoid(), ValueNumStore::VNForVoid()); } - else if ((tree->gtOp.gtOp2->OperGet() == GT_CLS_VAR) && - (tree->gtOp.gtOp2->gtFlags & GTF_CLS_VAR_ASG_LHS)) + else if ((op2->OperGet() == GT_CLS_VAR) && (op2->gtFlags & GTF_CLS_VAR_ASG_LHS)) { // If op2 represents the lhs of an assignment then we give a VNForVoid for the lhs op2vnp = ValueNumPair(ValueNumStore::VNForVoid(), ValueNumStore::VNForVoid()); } else { - vnStore->VNPUnpackExc(tree->gtOp.gtOp2->gtVNPair, &op2vnp, &op2Xvnp); + vnStore->VNPUnpackExc(op2->gtVNPair, &op2vnp, &op2Xvnp); } tree->gtVNPair = vnStore->VNPWithExc(op2vnp, vnStore->VNPExcSetUnion(op1Xvnp, op2Xvnp)); -- 2.7.4