From f1d83337137f335ffcf35ccd13166be5d579c3d1 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 22 Aug 2022 14:03:22 -0700 Subject: [PATCH 1/7] JIT: extend copy prop to local fields Allow copy prop to update GT_LCL_FLD nodes. Update local assertion gen for block opts to use the pre-morph tree to generate copy or zero assertions, since the semantics of the post-morph tree are often obscured by the copy/zero expansions. --- src/coreclr/jit/assertionprop.cpp | 82 +++++++++++++++++++++++++++++++ src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/morph.cpp | 8 +++ 3 files changed, 91 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e1f24367d17eae..8c9a0867442f86 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3568,6 +3568,20 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion, return nullptr; } + // Heuristic: for LclFld prop, don't force the copy or its promoted fields to be in memory. + // + if (tree->OperIs(GT_LCL_FLD)) + { + if (copyVarDsc->IsEnregisterableLcl() || copyVarDsc->lvPromotedStruct()) + { + return nullptr; + } + else + { + lvaSetVarDoNotEnregister(copyLclNum DEBUGARG(DoNotEnregisterReason::LocalField)); + } + } + tree->SetLclNum(copyLclNum); tree->SetSsaNum(copySsaNum); @@ -3689,6 +3703,71 @@ GenTree* Compiler::optAssertionProp_LclVar(ASSERT_VALARG_TP assertions, GenTreeL return nullptr; } +//------------------------------------------------------------------------ +// optAssertionProp_LclFld: try and optimize a local field use via assertions +// +// Arguments: +// assertions - set of live assertions +// tree - local field use to optimize +// stmt - statement containing the tree +// +// Returns: +// Updated tree, or nullptr +// +// Notes: +// stmt may be nullptr during local assertion prop +// +GenTree* Compiler::optAssertionProp_LclFld(ASSERT_VALARG_TP assertions, GenTreeLclVarCommon* tree, Statement* stmt) +{ + // If we have a var definition then bail or + // If this is the address of the var then it will have the GTF_DONT_CSE + // flag set and we don't want to to assertion prop on it. + if (tree->gtFlags & (GTF_VAR_DEF | GTF_DONT_CSE)) + { + return nullptr; + } + + // Only run during local prop and if copies are available. + // + if (!optLocalAssertionProp || !optCanPropLclVar) + { + return nullptr; + } + + BitVecOps::Iter iter(apTraits, assertions); + unsigned index = 0; + while (iter.NextElem(&index)) + { + AssertionIndex assertionIndex = GetAssertionIndex(index); + if (assertionIndex > optAssertionCount) + { + break; + } + + // See if the variable is equal to another variable. + AssertionDsc* curAssertion = optGetAssertion(assertionIndex); + if (!curAssertion->CanPropLclVar()) + { + continue; + } + + // Copy prop. + if (curAssertion->op2.kind == O2K_LCLVAR_COPY) + { + // Perform copy assertion prop. + GenTree* newTree = optCopyAssertionProp(curAssertion, tree, stmt DEBUGARG(assertionIndex)); + if (newTree != nullptr) + { + return newTree; + } + } + + continue; + } + + return nullptr; +} + //------------------------------------------------------------------------ // optAssertionProp_Asg: Try and optimize an assignment via assertions. // @@ -4916,6 +4995,9 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree, case GT_LCL_VAR: return optAssertionProp_LclVar(assertions, tree->AsLclVarCommon(), stmt); + case GT_LCL_FLD: + return optAssertionProp_LclFld(assertions, tree->AsLclVarCommon(), stmt); + case GT_ASG: return optAssertionProp_Asg(assertions, tree->AsOp(), stmt); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f1463351a86251..20b4ef705a5b50 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7358,6 +7358,7 @@ class Compiler // Assertion propagation functions. GenTree* optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt, BasicBlock* block); GenTree* optAssertionProp_LclVar(ASSERT_VALARG_TP assertions, GenTreeLclVarCommon* tree, Statement* stmt); + GenTree* optAssertionProp_LclFld(ASSERT_VALARG_TP assertions, GenTreeLclVarCommon* tree, Statement* stmt); GenTree* optAssertionProp_Asg(ASSERT_VALARG_TP assertions, GenTreeOp* asg, Statement* stmt); GenTree* optAssertionProp_Return(ASSERT_VALARG_TP assertions, GenTreeUnOp* ret, Statement* stmt); GenTree* optAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ff657de745a7ce..04fbe4c36147ce 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14393,6 +14393,14 @@ void Compiler::fgMorphTreeDone(GenTree* tree, /* If this tree makes a new assertion - make it available */ optAssertionGen(tree); + // For struct copies & inits, also use the original tree + // to generate assertions. + // + if ((oldTree != nullptr) && oldTree->OperIsBlkOp()) + { + optAssertionGen(oldTree); + } + DONE:; #ifdef DEBUG From aafa84b9b962948321a225797dd62cf649b2e548 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 23 Aug 2022 19:45:22 -0700 Subject: [PATCH 2/7] fix x86 issue --- src/coreclr/jit/assertionprop.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 8c9a0867442f86..bb8decf3593efe 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3416,19 +3416,33 @@ bool Compiler::optZeroObjAssertionProp(GenTree* tree, ASSERT_VALARG_TP assertion return false; } - unsigned lclNum = tree->AsLclVar()->GetLclNum(); + const unsigned lclNum = tree->AsLclVar()->GetLclNum(); AssertionIndex assertionIndex = optLocalAssertionIsEqualOrNotEqual(O1K_LCLVAR, lclNum, O2K_ZEROOBJ, 0, assertions); if (assertionIndex == NO_ASSERTION_INDEX) { return false; } + // TODO: create proper simd zero constant + // + if (varTypeIsSIMD(tree)) + { + return false; + } + AssertionDsc* assertion = optGetAssertion(assertionIndex); JITDUMP("\nAssertion prop in " FMT_BB ":\n", compCurBB->bbNum); JITDUMPEXEC(optPrintAssertion(assertion, assertionIndex)); DISPNODE(tree); - tree->BashToZeroConst(TYP_INT); + if (varTypeIsStruct(tree)) + { + tree->BashToZeroConst(TYP_INT); + } + else + { + tree->BashToZeroConst(tree->TypeGet()); + } JITDUMP(" =>\n"); DISPNODE(tree); From bd69556c87b26e3c4e58b655f3ce7e5a13e3a878 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 23 Aug 2022 07:49:49 -0700 Subject: [PATCH 3/7] alternate version where we gen during morph block --- src/coreclr/jit/morph.cpp | 16 ++++++---------- src/coreclr/jit/morphblock.cpp | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 04fbe4c36147ce..db67b1ed3de082 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14382,9 +14382,13 @@ void Compiler::fgMorphTreeDone(GenTree* tree, // DefinesLocal can return true for some BLK op uses, so // check what gets assigned only when we're at an assignment. - if (tree->OperIsSsaDef() && tree->DefinesLocal(this, &lclVarTree)) + // + // If the tree is a block op we've already done the kills + // over in MorphInitBlockHelper::PrepareDst(). + // + if (tree->OperIsSsaDef() && !tree->OperIsBlk() && tree->DefinesLocal(this, &lclVarTree)) { - unsigned lclNum = lclVarTree->GetLclNum(); + const unsigned lclNum = lclVarTree->GetLclNum(); noway_assert(lclNum < lvaCount); fgKillDependentAssertions(lclNum DEBUGARG(tree)); } @@ -14393,14 +14397,6 @@ void Compiler::fgMorphTreeDone(GenTree* tree, /* If this tree makes a new assertion - make it available */ optAssertionGen(tree); - // For struct copies & inits, also use the original tree - // to generate assertions. - // - if ((oldTree != nullptr) && oldTree->OperIsBlkOp()) - { - optAssertionGen(oldTree); - } - DONE:; #ifdef DEBUG diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index a976ded54d7f83..6cac7576794520 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -19,6 +19,8 @@ class MorphInitBlockHelper virtual void TrySpecialCases(); virtual void MorphStructCases(); + void PropagateAssertions(); + virtual const char* GetHelperName() const { return "MorphInitBlock"; @@ -125,7 +127,7 @@ GenTree* MorphInitBlockHelper::Morph() PrepareDst(); PrepareSrc(); - + PropagateAssertions(); TrySpecialCases(); if (m_transformationDecision == BlockTransformation::Undefined) @@ -275,6 +277,23 @@ void MorphInitBlockHelper::PrepareDst() #endif // DEBUG } +//------------------------------------------------------------------------ +// PropagateAssertions: propagate assertions based on the original tree +// +// Notes: +// Once the init or copy tree is morphed, assertion gen can no +// longer recognize what it means. +// +// So we generate assertions based on the original tree. +// +void MorphInitBlockHelper::PropagateAssertions() +{ + if (m_comp->optLocalAssertionProp) + { + m_comp->optAssertionGen(m_asg); + } +} + //------------------------------------------------------------------------ // PrepareSrc: Transform the asg src to an appropriate form and initialize member fields // with information about it. From 036011915eb62cce0728e9a55ccd1b6dfbbb9a15 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 23 Aug 2022 23:33:03 -0700 Subject: [PATCH 4/7] plumb flag through to decide what to do in fgMorphTreeDone --- src/coreclr/jit/assertionprop.cpp | 21 +++++ src/coreclr/jit/compiler.h | 6 +- src/coreclr/jit/morph.cpp | 135 ++++++++++++++++-------------- src/coreclr/jit/morphblock.cpp | 2 +- 4 files changed, 97 insertions(+), 67 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index bb8decf3593efe..919c563c981019 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1636,8 +1636,29 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, // // Copy Assertions // + case GT_OBJ: + case GT_BLK: + { + GenTree* const addr = op2->AsIndir()->Addr(); + + if (addr->OperIs(GT_ADDR)) + { + GenTree* const base = addr->AsOp()->gtOp1; + + if (base->OperIs(GT_LCL_VAR)) + { + // layout compat? + op2 = base; + goto IS_COPY; + } + } + + goto DONE_ASSERTION; + } + case GT_LCL_VAR: { + IS_COPY: // // Must either be an OAK_EQUAL or an OAK_NOT_EQUAL assertion // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 20b4ef705a5b50..100f275830d878 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5758,7 +5758,7 @@ class Compiler GenTree* fgMorphCopyBlock(GenTree* tree); GenTree* fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree); GenTree* fgMorphForRegisterFP(GenTree* tree); - GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac = nullptr); + GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optAssertionPropDone = nullptr); GenTree* fgOptimizeCast(GenTreeCast* cast); GenTree* fgOptimizeCastOnAssignment(GenTreeOp* asg); GenTree* fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp); @@ -5777,7 +5777,7 @@ class Compiler GenTree* fgMorphRetInd(GenTreeUnOp* tree); GenTree* fgMorphModToSubMulDiv(GenTreeOp* tree); GenTree* fgMorphUModToAndSub(GenTreeOp* tree); - GenTree* fgMorphSmpOpOptional(GenTreeOp* tree); + GenTree* fgMorphSmpOpOptional(GenTreeOp* tree, bool* optAssertionPropDone); GenTree* fgMorphMultiOp(GenTreeMultiOp* multiOp); GenTree* fgMorphConst(GenTree* tree); @@ -5793,7 +5793,7 @@ class Compiler private: void fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree* tree)); void fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree)); - void fgMorphTreeDone(GenTree* tree, GenTree* oldTree = nullptr DEBUGARG(int morphNum = 0)); + void fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone = false DEBUGARG(int morphNum = 0)); Statement* fgMorphStmt; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index db67b1ed3de082..31342dd7e59849 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5499,7 +5499,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); } - return fgMorphSmpOp(tree); + return fgMorphSmpOp(tree, /* mac */ nullptr); } } @@ -9770,16 +9770,23 @@ GenTree* Compiler::fgMorphCastedBitwiseOp(GenTreeOp* tree) return nullptr; } -/***************************************************************************** - * - * Transform the given GTK_SMPOP tree for code generation. - */ - +//------------------------------------------------------------------------ +// fgMorphSmpOp: morph a GTK_SMPOP tree +// +// Arguments: +// tree - tree to morph +// mac - address context for morphing +// optAssertionPropDone - [out, optional] set true if local assertions +// were killed/genned while morphing this tree +// +// Returns: +// Tree, possibly updated +// #ifdef _PREFAST_ #pragma warning(push) #pragma warning(disable : 21000) // Suppress PREFast warning about overly large function #endif -GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) +GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optAssertionPropDone) { ALLOCA_CHECK(); assert(tree->OperKind() & GTK_SMPOP); @@ -11712,7 +11719,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return tree; } - tree = fgMorphSmpOpOptional(tree->AsOp()); + tree = fgMorphSmpOpOptional(tree->AsOp(), optAssertionPropDone); return tree; } @@ -13098,8 +13105,18 @@ GenTree* Compiler::fgMorphRetInd(GenTreeUnOp* ret) #ifdef _PREFAST_ #pragma warning(pop) #endif - -GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree) +//------------------------------------------------------------- +// fgMorphSmpOpOptional: optional post-order morping of some SMP trees +// +// Arguments: +// tree - tree to morph +// optAssertionPropDone - [out, optional] set true if local assertions were +// killed/genned by the optional morphing +// +// Returns: +// Tree, possibly updated +// +GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree, bool* optAssertionPropDone) { genTreeOps oper = tree->gtOper; GenTree* op1 = tree->gtOp1; @@ -13198,6 +13215,14 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree) if (varTypeIsStruct(typ) && !tree->IsPhiDefn()) { + // Block ops handle assertion kill/gen specially. + // See PrepareDst and PropagateAssertions + // + if (optAssertionPropDone != nullptr) + { + *optAssertionPropDone = true; + } + if (tree->OperIsCopyBlkOp()) { return fgMorphCopyBlock(tree); @@ -14018,6 +14043,8 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) } #endif + bool optAssertionPropDone = false; + /*------------------------------------------------------------------------- * fgMorphTree() can potentially replace a tree with another, and the * caller has to store the return value correctly. @@ -14080,10 +14107,6 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) PREFAST_ASSUME(tree != nullptr); } - /* Save the original un-morphed tree for fgMorphTreeDone */ - - GenTree* oldTree = tree; - /* Figure out what kind of a node we have */ unsigned kind = tree->OperKind(); @@ -14108,7 +14131,7 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) if (kind & GTK_SMPOP) { - tree = fgMorphSmpOp(tree, mac); + tree = fgMorphSmpOp(tree, mac, &optAssertionPropDone); goto DONE; } @@ -14220,7 +14243,7 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) } DONE: - fgMorphTreeDone(tree, oldTree DEBUGARG(thisMorphNum)); + fgMorphTreeDone(tree, optAssertionPropDone DEBUGARG(thisMorphNum)); return tree; } @@ -14313,20 +14336,24 @@ void Compiler::fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree) } } -/***************************************************************************** - * - * This function is called to complete the morphing of a tree node - * It should only be called once for each node. - * If DEBUG is defined the flag GTF_DEBUG_NODE_MORPHED is checked and updated, - * to enforce the invariant that each node is only morphed once. - * If local assertion prop is enabled the result tree may be replaced - * by an equivalent tree. - * - */ - -void Compiler::fgMorphTreeDone(GenTree* tree, - GenTree* oldTree /* == NULL */ - DEBUGARG(int morphNum)) +//------------------------------------------------------------------------ +// fgMorphTreeDone: complete the morphing of a tree node +// +// Arguments: +// tree - the tree after morphing +// optAssertionPropDone - true if local assertion prop was done already +// morphNum - counts invocations of fgMorphTree +// +// Notes: +// This function is called to complete the morphing of a tree node +// It should only be called once for each node. +// If DEBUG is defined the flag GTF_DEBUG_NODE_MORPHED is checked and updated, +// to enforce the invariant that each node is only morphed once. +// +// When local assertion prop is active assertions are killed and generated +// based on tree (unless optAssertionPropDone is true). +// +void Compiler::fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG(int morphNum)) { #ifdef DEBUG if (verbose && treesBeforeAfterMorph) @@ -14342,36 +14369,27 @@ void Compiler::fgMorphTreeDone(GenTree* tree, return; } - if ((oldTree != nullptr) && (oldTree != tree)) - { - /* Ensure that we have morphed this node */ - assert((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) && "ERROR: Did not morph this node!"); - #ifdef DEBUG - TransferTestDataToNode(oldTree, tree); -#endif - } - else + if ((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) { - // Ensure that we haven't morphed this node already - assert(((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) && "ERROR: Already morphed this node!"); - } - - if (tree->OperIsConst()) - { - goto DONE; + assert("ERROR: Did not morph this node!"); } + tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; +#endif - if (!optLocalAssertionProp) + // note "tree" may generate new assertions that we + // miss if we do them early... perhaps we should skip + // kills but rerun gens. + // + if (tree->OperIsConst() || !optLocalAssertionProp || optAssertionPropDone) { - goto DONE; + return; } - /* Do we have any active assertions? */ - + // Kill active assertions + // if (optAssertionCount > 0) { - /* Is this an assignment to a local variable */ GenTreeLclVarCommon* lclVarTree = nullptr; // The check below will miss LIR-style assignments. @@ -14383,10 +14401,7 @@ void Compiler::fgMorphTreeDone(GenTree* tree, // DefinesLocal can return true for some BLK op uses, so // check what gets assigned only when we're at an assignment. // - // If the tree is a block op we've already done the kills - // over in MorphInitBlockHelper::PrepareDst(). - // - if (tree->OperIsSsaDef() && !tree->OperIsBlk() && tree->DefinesLocal(this, &lclVarTree)) + if (tree->OperIsSsaDef() && tree->DefinesLocal(this, &lclVarTree)) { const unsigned lclNum = lclVarTree->GetLclNum(); noway_assert(lclNum < lvaCount); @@ -14394,15 +14409,9 @@ void Compiler::fgMorphTreeDone(GenTree* tree, } } - /* If this tree makes a new assertion - make it available */ + // Generate new assertions + // optAssertionGen(tree); - -DONE:; - -#ifdef DEBUG - /* Mark this node as being morphed */ - tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; -#endif } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 6cac7576794520..d3db65e1785c37 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -281,7 +281,7 @@ void MorphInitBlockHelper::PrepareDst() // PropagateAssertions: propagate assertions based on the original tree // // Notes: -// Once the init or copy tree is morphed, assertion gen can no +// Once the init or copy tree is morphed, assertion gen can no // longer recognize what it means. // // So we generate assertions based on the original tree. From a5ea0ebefed8cf10be2440da6bb94e43fd81f4a8 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 24 Aug 2022 21:45:27 -0700 Subject: [PATCH 5/7] First bit of feedback. Still producing SIMD ZEROOBJ but no longer looking for them. --- src/coreclr/jit/assertionprop.cpp | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 919c563c981019..5d9677b00b6607 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3437,16 +3437,16 @@ bool Compiler::optZeroObjAssertionProp(GenTree* tree, ASSERT_VALARG_TP assertion return false; } - const unsigned lclNum = tree->AsLclVar()->GetLclNum(); - AssertionIndex assertionIndex = optLocalAssertionIsEqualOrNotEqual(O1K_LCLVAR, lclNum, O2K_ZEROOBJ, 0, assertions); - if (assertionIndex == NO_ASSERTION_INDEX) + // No ZEROOBJ assertions for simd. + // + if (varTypeIsSIMD(tree)) { return false; } - // TODO: create proper simd zero constant - // - if (varTypeIsSIMD(tree)) + const unsigned lclNum = tree->AsLclVar()->GetLclNum(); + AssertionIndex assertionIndex = optLocalAssertionIsEqualOrNotEqual(O1K_LCLVAR, lclNum, O2K_ZEROOBJ, 0, assertions); + if (assertionIndex == NO_ASSERTION_INDEX) { return false; } @@ -3456,14 +3456,7 @@ bool Compiler::optZeroObjAssertionProp(GenTree* tree, ASSERT_VALARG_TP assertion JITDUMPEXEC(optPrintAssertion(assertion, assertionIndex)); DISPNODE(tree); - if (varTypeIsStruct(tree)) - { - tree->BashToZeroConst(TYP_INT); - } - else - { - tree->BashToZeroConst(tree->TypeGet()); - } + tree->BashToZeroConst(TYP_INT); JITDUMP(" =>\n"); DISPNODE(tree); From 813f22e124bb97033d5298d9be8ca98932cc1f23 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 24 Aug 2022 22:57:40 -0700 Subject: [PATCH 6/7] restore GTF_DEBUG_NODE_MORPHED checks --- src/coreclr/jit/compiler.h | 3 ++- src/coreclr/jit/morph.cpp | 47 ++++++++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 100f275830d878..92f18698d4556c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5793,7 +5793,8 @@ class Compiler private: void fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree* tree)); void fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree)); - void fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone = false DEBUGARG(int morphNum = 0)); + void fgMorphTreeDone(GenTree* tree); + void fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone, bool isMorphedTree DEBUGARG(int morphNum = 0)); Statement* fgMorphStmt; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 31342dd7e59849..02f0b9a6383471 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14107,9 +14107,13 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) PREFAST_ASSUME(tree != nullptr); } + /* Save the original un-morphed tree for fgMorphTreeDone */ + + GenTree* const oldTree = tree; + /* Figure out what kind of a node we have */ - unsigned kind = tree->OperKind(); + unsigned const kind = tree->OperKind(); /* Is this a constant node? */ @@ -14243,7 +14247,8 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) } DONE: - fgMorphTreeDone(tree, optAssertionPropDone DEBUGARG(thisMorphNum)); + const bool isNewTree = (oldTree != tree); + fgMorphTreeDone(tree, optAssertionPropDone, isNewTree DEBUGARG(thisMorphNum)); return tree; } @@ -14336,12 +14341,28 @@ void Compiler::fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree) } } +//------------------------------------------------------------------------ +// fgMorphTreeDone: complete the morphing of a tree node +// +// Arguments: +// tree - the tree after morphing +// +// Notes: +// Simple version where the tree has not been marked +// as morphed, and where assertion kill/gen has not yet been done. +// +void Compiler::fgMorphTreeDone(GenTree* tree) +{ + fgMorphTreeDone(tree, false, false); +} + //------------------------------------------------------------------------ // fgMorphTreeDone: complete the morphing of a tree node // // Arguments: // tree - the tree after morphing // optAssertionPropDone - true if local assertion prop was done already +// isMorphedTree - true if caller should have marked tree as morphed // morphNum - counts invocations of fgMorphTree // // Notes: @@ -14353,7 +14374,7 @@ void Compiler::fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree) // When local assertion prop is active assertions are killed and generated // based on tree (unless optAssertionPropDone is true). // -void Compiler::fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG(int morphNum)) +void Compiler::fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone, bool isMorphedTree DEBUGARG(int morphNum)) { #ifdef DEBUG if (verbose && treesBeforeAfterMorph) @@ -14369,16 +14390,22 @@ void Compiler::fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG return; } -#ifdef DEBUG - if ((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) + if (isMorphedTree) { - assert("ERROR: Did not morph this node!"); + // caller should have set the morphed flag + // + assert((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) && "ERROR: Did not morph this node!"); + } + else + { + // caller should not have set the morphed flag + // + assert(((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) && "ERROR: Already morphed this node!"); + INDEBUG(tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); } - tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; -#endif - // note "tree" may generate new assertions that we - // miss if we do them early... perhaps we should skip + // Note "tree" may generate new assertions that we + // miss if we did them early... perhaps we should skip // kills but rerun gens. // if (tree->OperIsConst() || !optLocalAssertionProp || optAssertionPropDone) From 752e70162466837e3e8397c7e26fe056a2094447 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Aug 2022 08:42:39 -0700 Subject: [PATCH 7/7] add comment and layout check to optCreateAssertion --- src/coreclr/jit/assertionprop.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 0c971d873e791a..a066b23403609a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1639,17 +1639,23 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, case GT_OBJ: case GT_BLK: { + // TODO-ADDR: delete once local morph folds SIMD-typed indirections. + // GenTree* const addr = op2->AsIndir()->Addr(); if (addr->OperIs(GT_ADDR)) { GenTree* const base = addr->AsOp()->gtOp1; - if (base->OperIs(GT_LCL_VAR)) + if (base->OperIs(GT_LCL_VAR) && varTypeIsStruct(base)) { - // layout compat? - op2 = base; - goto IS_COPY; + ClassLayout* const varLayout = base->GetLayout(this); + ClassLayout* const objLayout = op2->GetLayout(this); + if (ClassLayout::AreCompatible(varLayout, objLayout)) + { + op2 = base; + goto IS_COPY; + } } }