From c2c92c4cf3e650d192cc39da89ba130aef698bfa Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 9 Jun 2021 16:50:38 -0700 Subject: [PATCH 1/6] Do not mark op2 as delayRegFree if op1==op2 --- src/coreclr/jit/gentree.cpp | 49 +++++++++++++++++++++++++++++++ src/coreclr/jit/gentree.h | 2 ++ src/coreclr/jit/lower.cpp | 55 ++--------------------------------- src/coreclr/jit/lower.h | 2 -- src/coreclr/jit/lsraxarch.cpp | 11 +++++-- 5 files changed, 63 insertions(+), 56 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index e629d1a4b83ade..32f415f4840770 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -16921,6 +16921,55 @@ ssize_t GenTreeIndir::Offset() } } +/** Test whether the two given nodes are the same leaves. + * Right now, only constant integers and local variables are supported + */ +bool GenTree::NodesAreEquivalentLeaves(GenTree* tree1, GenTree* tree2) +{ + if (tree1 == nullptr && tree2 == nullptr) + { + return true; + } + + // both null, they are equivalent, otherwise if either is null not equivalent + if (tree1 == nullptr || tree2 == nullptr) + { + return false; + } + + tree1 = tree1->gtSkipReloadOrCopy(); + tree2 = tree2->gtSkipReloadOrCopy(); + + if (tree1->TypeGet() != tree2->TypeGet()) + { + return false; + } + + if (tree1->OperGet() != tree2->OperGet()) + { + return false; + } + + if (!tree1->OperIsLeaf() || !tree2->OperIsLeaf()) + { + return false; + } + + switch (tree1->OperGet()) + { + case GT_CNS_INT: + return tree1->AsIntCon()->gtIconVal == tree2->AsIntCon()->gtIconVal && + tree1->IsIconHandle() == tree2->IsIconHandle(); + case GT_LCL_VAR: + case GT_LCL_VAR_ADDR: + return tree1->AsLclVarCommon()->GetLclNum() == tree2->AsLclVarCommon()->GetLclNum(); + case GT_CLS_VAR_ADDR: + return tree1->AsClsVar()->gtClsVarHnd == tree2->AsClsVar()->gtClsVarHnd; + default: + return false; + } +} + //------------------------------------------------------------------------ // GenTreeIntConCommon::ImmedValNeedsReloc: does this immediate value needs recording a relocation with the VM? // diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ef65f7d7aae051..5c66cfc7b76cdf 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -850,6 +850,8 @@ struct GenTree _gtCostSz = tree->_gtCostSz; } + static bool NodesAreEquivalentLeaves(GenTree* candidate, GenTree* storeInd); + private: unsigned char _gtCostEx; // estimate of expression execution cost unsigned char _gtCostSz; // estimate of expression code size cost diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 254d557b188344..bd478b4ef73608 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6193,14 +6193,14 @@ bool Lowering::IndirsAreEquivalent(GenTree* candidate, GenTree* storeInd) case GT_LCL_VAR_ADDR: case GT_CLS_VAR_ADDR: case GT_CNS_INT: - return NodesAreEquivalentLeaves(pTreeA, pTreeB); + return GenTree::NodesAreEquivalentLeaves(pTreeA, pTreeB); case GT_LEA: { GenTreeAddrMode* gtAddr1 = pTreeA->AsAddrMode(); GenTreeAddrMode* gtAddr2 = pTreeB->AsAddrMode(); - return NodesAreEquivalentLeaves(gtAddr1->Base(), gtAddr2->Base()) && - NodesAreEquivalentLeaves(gtAddr1->Index(), gtAddr2->Index()) && + return GenTree::NodesAreEquivalentLeaves(gtAddr1->Base(), gtAddr2->Base()) && + GenTree::NodesAreEquivalentLeaves(gtAddr1->Index(), gtAddr2->Index()) && (gtAddr1->gtScale == gtAddr2->gtScale) && (gtAddr1->Offset() == gtAddr2->Offset()); } default: @@ -6210,55 +6210,6 @@ bool Lowering::IndirsAreEquivalent(GenTree* candidate, GenTree* storeInd) } } -/** Test whether the two given nodes are the same leaves. - * Right now, only constant integers and local variables are supported - */ -bool Lowering::NodesAreEquivalentLeaves(GenTree* tree1, GenTree* tree2) -{ - if (tree1 == nullptr && tree2 == nullptr) - { - return true; - } - - // both null, they are equivalent, otherwise if either is null not equivalent - if (tree1 == nullptr || tree2 == nullptr) - { - return false; - } - - tree1 = tree1->gtSkipReloadOrCopy(); - tree2 = tree2->gtSkipReloadOrCopy(); - - if (tree1->TypeGet() != tree2->TypeGet()) - { - return false; - } - - if (tree1->OperGet() != tree2->OperGet()) - { - return false; - } - - if (!tree1->OperIsLeaf() || !tree2->OperIsLeaf()) - { - return false; - } - - switch (tree1->OperGet()) - { - case GT_CNS_INT: - return tree1->AsIntCon()->gtIconVal == tree2->AsIntCon()->gtIconVal && - tree1->IsIconHandle() == tree2->IsIconHandle(); - case GT_LCL_VAR: - case GT_LCL_VAR_ADDR: - return tree1->AsLclVarCommon()->GetLclNum() == tree2->AsLclVarCommon()->GetLclNum(); - case GT_CLS_VAR_ADDR: - return tree1->AsClsVar()->gtClsVarHnd == tree2->AsClsVar()->gtClsVarHnd; - default: - return false; - } -} - //------------------------------------------------------------------------ // Lowering::CheckMultiRegLclVar: Check whether a MultiReg GT_LCL_VAR node can // remain a multi-reg. diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 7f7c9d3760aa14..e28f0a767daf07 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -569,8 +569,6 @@ class Lowering final : public Phase static void TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, BasicBlock* block); private: - static bool NodesAreEquivalentLeaves(GenTree* candidate, GenTree* storeInd); - bool AreSourcesPossiblyModifiedLocals(GenTree* addr, GenTree* base, GenTree* index); // Makes 'childNode' contained in the 'parentNode' diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 1cd81124d05663..b11ae1870a08e2 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -2481,8 +2481,15 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) // When op2 is not contained or if we are producing a scalar value // we need to mark it as delay free because the operand and target // exist in the same register set. - - srcCount += BuildDelayFreeUses(op2); + // Unless, op1 and op2 are same, in which case we can overwrite op2. + if (GenTree::NodesAreEquivalentLeaves(op1, op2)) + { + srcCount += BuildOperandUses(op2); + } + else + { + srcCount += BuildDelayFreeUses(op2); + } } else { From dee1a09815a54c8a841069b0f3a9677041c57aba Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 10 Jun 2021 17:33:47 -0700 Subject: [PATCH 2/6] Revert NodesAreEquivalentLeaves change --- src/coreclr/jit/gentree.cpp | 49 ---------------------------- src/coreclr/jit/gentree.h | 2 -- src/coreclr/jit/lower.cpp | 60 +++++++++++++++++++++++++++++++++-- src/coreclr/jit/lower.h | 2 ++ src/coreclr/jit/lsrabuild.cpp | 4 +++ 5 files changed, 63 insertions(+), 54 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 32f415f4840770..e629d1a4b83ade 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -16921,55 +16921,6 @@ ssize_t GenTreeIndir::Offset() } } -/** Test whether the two given nodes are the same leaves. - * Right now, only constant integers and local variables are supported - */ -bool GenTree::NodesAreEquivalentLeaves(GenTree* tree1, GenTree* tree2) -{ - if (tree1 == nullptr && tree2 == nullptr) - { - return true; - } - - // both null, they are equivalent, otherwise if either is null not equivalent - if (tree1 == nullptr || tree2 == nullptr) - { - return false; - } - - tree1 = tree1->gtSkipReloadOrCopy(); - tree2 = tree2->gtSkipReloadOrCopy(); - - if (tree1->TypeGet() != tree2->TypeGet()) - { - return false; - } - - if (tree1->OperGet() != tree2->OperGet()) - { - return false; - } - - if (!tree1->OperIsLeaf() || !tree2->OperIsLeaf()) - { - return false; - } - - switch (tree1->OperGet()) - { - case GT_CNS_INT: - return tree1->AsIntCon()->gtIconVal == tree2->AsIntCon()->gtIconVal && - tree1->IsIconHandle() == tree2->IsIconHandle(); - case GT_LCL_VAR: - case GT_LCL_VAR_ADDR: - return tree1->AsLclVarCommon()->GetLclNum() == tree2->AsLclVarCommon()->GetLclNum(); - case GT_CLS_VAR_ADDR: - return tree1->AsClsVar()->gtClsVarHnd == tree2->AsClsVar()->gtClsVarHnd; - default: - return false; - } -} - //------------------------------------------------------------------------ // GenTreeIntConCommon::ImmedValNeedsReloc: does this immediate value needs recording a relocation with the VM? // diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 5c66cfc7b76cdf..ef65f7d7aae051 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -850,8 +850,6 @@ struct GenTree _gtCostSz = tree->_gtCostSz; } - static bool NodesAreEquivalentLeaves(GenTree* candidate, GenTree* storeInd); - private: unsigned char _gtCostEx; // estimate of expression execution cost unsigned char _gtCostSz; // estimate of expression code size cost diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index bd478b4ef73608..389c29f7643056 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6193,14 +6193,14 @@ bool Lowering::IndirsAreEquivalent(GenTree* candidate, GenTree* storeInd) case GT_LCL_VAR_ADDR: case GT_CLS_VAR_ADDR: case GT_CNS_INT: - return GenTree::NodesAreEquivalentLeaves(pTreeA, pTreeB); + return NodesAreEquivalentLeaves(pTreeA, pTreeB); case GT_LEA: { GenTreeAddrMode* gtAddr1 = pTreeA->AsAddrMode(); GenTreeAddrMode* gtAddr2 = pTreeB->AsAddrMode(); - return GenTree::NodesAreEquivalentLeaves(gtAddr1->Base(), gtAddr2->Base()) && - GenTree::NodesAreEquivalentLeaves(gtAddr1->Index(), gtAddr2->Index()) && + return NodesAreEquivalentLeaves(gtAddr1->Base(), gtAddr2->Base()) && + NodesAreEquivalentLeaves(gtAddr1->Index(), gtAddr2->Index()) && (gtAddr1->gtScale == gtAddr2->gtScale) && (gtAddr1->Offset() == gtAddr2->Offset()); } default: @@ -6210,6 +6210,60 @@ bool Lowering::IndirsAreEquivalent(GenTree* candidate, GenTree* storeInd) } } +//------------------------------------------------------------------------ +// NodesAreEquivalentLeaves: Check whether the two given nodes are the same leaves. +// +// Arguments: +// tree1 and tree2 are nodes to be checked. +// Return Value: +// Returns true if they are same leaves, false otherwise. +// +// static +bool Lowering::NodesAreEquivalentLeaves(GenTree* tree1, GenTree* tree2) +{ + if (tree1 == tree2) + { + return true; + } + + if (tree1 == nullptr || tree2 == nullptr) + { + return false; + } + + tree1 = tree1->gtSkipReloadOrCopy(); + tree2 = tree2->gtSkipReloadOrCopy(); + + if (tree1->TypeGet() != tree2->TypeGet()) + { + return false; + } + + if (tree1->OperGet() != tree2->OperGet()) + { + return false; + } + + if (!tree1->OperIsLeaf() || !tree2->OperIsLeaf()) + { + return false; + } + + switch (tree1->OperGet()) + { + case GT_CNS_INT: + return tree1->AsIntCon()->IconValue() == tree2->AsIntCon()->IconValue() && + tree1->IsIconHandle() == tree2->IsIconHandle(); + case GT_LCL_VAR: + case GT_LCL_VAR_ADDR: + return tree1->AsLclVarCommon()->GetLclNum() == tree2->AsLclVarCommon()->GetLclNum(); + case GT_CLS_VAR_ADDR: + return tree1->AsClsVar()->gtClsVarHnd == tree2->AsClsVar()->gtClsVarHnd; + default: + return false; + } +} + //------------------------------------------------------------------------ // Lowering::CheckMultiRegLclVar: Check whether a MultiReg GT_LCL_VAR node can // remain a multi-reg. diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index e28f0a767daf07..7f7c9d3760aa14 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -569,6 +569,8 @@ class Lowering final : public Phase static void TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, BasicBlock* block); private: + static bool NodesAreEquivalentLeaves(GenTree* candidate, GenTree* storeInd); + bool AreSourcesPossiblyModifiedLocals(GenTree* addr, GenTree* base, GenTree* index); // Makes 'childNode' contained in the 'parentNode' diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 536cfa4d1b0ad7..6a58601b1eeb25 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3097,6 +3097,10 @@ int LinearScan::BuildDelayFreeUses(GenTree* node, GenTree* rmwNode, regMaskTP ca } if (use != nullptr) { + // If node != rmwNode, then definitely node should be marked as "delayFree". + // However, if node == rmwNode, then we can mark node as "delayFree" only + // none of the node/rmwNode are the last use. If either of them are last use, + // we can safely reuse the rmwNode as destination. if ((use->getInterval() != rmwInterval) || (!rmwIsLastUse && !use->lastUse)) { setDelayFree(use); From a4512081872ffba795009dd262d7bfeda6f22e64 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 11 Jun 2021 14:52:21 -0700 Subject: [PATCH 3/6] Pass rmwNode to `BuildDelayFreeUses()` which does the right thing --- src/coreclr/jit/lsrabuild.cpp | 2 ++ src/coreclr/jit/lsraxarch.cpp | 18 +++++------------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 6a58601b1eeb25..f92ffa4b98b963 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3101,6 +3101,8 @@ int LinearScan::BuildDelayFreeUses(GenTree* node, GenTree* rmwNode, regMaskTP ca // However, if node == rmwNode, then we can mark node as "delayFree" only // none of the node/rmwNode are the last use. If either of them are last use, // we can safely reuse the rmwNode as destination. + // TODO: What happens to constants? Do they have same interval? + // TODO: Do we need `!use->lastUse`? if ((use->getInterval() != rmwInterval) || (!rmwIsLastUse && !use->lastUse)) { setDelayFree(use); diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index b11ae1870a08e2..811010f4f37365 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -2420,9 +2420,9 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) // Any pair of the index, mask, or destination registers should be different srcCount += BuildOperandUses(op1); - srcCount += BuildDelayFreeUses(op2); - srcCount += BuildDelayFreeUses(op3); - srcCount += BuildDelayFreeUses(op4); + srcCount += BuildDelayFreeUses(op2, op1); + srcCount += BuildDelayFreeUses(op3, op1); + srcCount += BuildDelayFreeUses(op4, op1); // op5 should always be contained assert(argList->Rest()->Current()->isContained()); @@ -2481,15 +2481,7 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) // When op2 is not contained or if we are producing a scalar value // we need to mark it as delay free because the operand and target // exist in the same register set. - // Unless, op1 and op2 are same, in which case we can overwrite op2. - if (GenTree::NodesAreEquivalentLeaves(op1, op2)) - { - srcCount += BuildOperandUses(op2); - } - else - { - srcCount += BuildDelayFreeUses(op2); - } + srcCount += BuildDelayFreeUses(op2, op1); } else { @@ -2507,7 +2499,7 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) if (op3 != nullptr) { - srcCount += isRMW ? BuildDelayFreeUses(op3) : BuildOperandUses(op3); + srcCount += isRMW ? BuildDelayFreeUses(op3, op1) : BuildOperandUses(op3); } } } From bcd1313866fae668247efd49803b02abbbb76597 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 11 Jun 2021 16:23:45 -0700 Subject: [PATCH 4/6] Make similar change in arm64 --- src/coreclr/jit/lsraarm64.cpp | 64 ++++++++++++----------------------- 1 file changed, 22 insertions(+), 42 deletions(-) diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 223dc906badbf2..d6349054953d51 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -1112,53 +1112,26 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) // RMW intrinsic operands doesn't have to be delayFree when they can be assigned the same register as op1Reg // (i.e. a register that corresponds to read-modify-write operand) and one of them is the last use. - bool op2DelayFree = isRMW; - bool op3DelayFree = isRMW; - bool op4DelayFree = isRMW; - assert(intrin.op1 != nullptr); - if (isRMW && intrin.op1->OperIs(GT_LCL_VAR)) + bool forceOp2DelayFree = false; + if ((intrin.id == NI_Vector64_GetElement) || (intrin.id == NI_Vector128_GetElement)) { - unsigned int varNum1 = intrin.op1->AsLclVar()->GetLclNum(); - bool op1LastUse = false; - - unsigned int varNum2 = BAD_VAR_NUM; - unsigned int varNum3 = BAD_VAR_NUM; - unsigned int varNum4 = BAD_VAR_NUM; - if (intrin.op2->OperIs(GT_LCL_VAR)) +#ifdef DEBUG + if (isRMW && intrin.op1->OperIs(GT_LCL_VAR)) { - varNum2 = intrin.op2->AsLclVar()->GetLclNum(); - op1LastUse |= ((varNum1 == varNum2) && intrin.op2->HasLastUse()); - } + unsigned int varNum1 = intrin.op1->AsLclVar()->GetLclNum(); + bool op1LastUse = false; - if (intrin.op3 != nullptr) - { - if (intrin.op3->OperIs(GT_LCL_VAR)) + unsigned int varNum2 = BAD_VAR_NUM; + if (intrin.op2->OperIs(GT_LCL_VAR)) { - varNum3 = intrin.op3->AsLclVar()->GetLclNum(); - op1LastUse |= ((varNum1 == varNum3) && intrin.op3->HasLastUse()); + varNum2 = intrin.op2->AsLclVar()->GetLclNum(); + assert((varNum1 == varNum2) && intrin.op2->HasLastUse()); } - - if ((intrin.op4 != nullptr) && intrin.op4->OperIs(GT_LCL_VAR)) - { - varNum4 = intrin.op4->AsLclVar()->GetLclNum(); - op1LastUse |= ((varNum1 == varNum4) && intrin.op4->HasLastUse()); - } - } - - if (op1LastUse) - { - op2DelayFree = (varNum1 != varNum2); - op3DelayFree = (varNum1 != varNum3); - op4DelayFree = (varNum1 != varNum4); } - } - - if ((intrin.id == NI_Vector64_GetElement) || (intrin.id == NI_Vector128_GetElement)) - { - assert(!op2DelayFree); +#endif if (!intrin.op2->IsCnsIntOrI() && (!intrin.op1->isContained() || intrin.op1->OperIsLocal())) { @@ -1168,7 +1141,7 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) // TODO-Cleanup: An internal register will never clobber a source; this code actually // ensures that the index (op2) doesn't interfere with the target. buildInternalIntRegisterDefForNode(intrinsicTree); - op2DelayFree = true; + forceOp2DelayFree = true; } if (!intrin.op2->IsCnsIntOrI() && !intrin.op1->isContained()) @@ -1179,15 +1152,22 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) } } - srcCount += op2DelayFree ? BuildDelayFreeUses(intrin.op2) : BuildOperandUses(intrin.op2); + if (forceOp2DelayFree) + { + srcCount += BuildDelayFreeUses(intrin.op2); + } + else + { + srcCount += isRMW ? BuildDelayFreeUses(intrin.op2, intrin.op1) : BuildOperandUses(intrin.op2); + } if (intrin.op3 != nullptr) { - srcCount += op3DelayFree ? BuildDelayFreeUses(intrin.op3) : BuildOperandUses(intrin.op3); + srcCount += isRMW ? BuildDelayFreeUses(intrin.op3, intrin.op1) : BuildOperandUses(intrin.op3); if (intrin.op4 != nullptr) { - srcCount += op4DelayFree ? BuildDelayFreeUses(intrin.op4) : BuildOperandUses(intrin.op4); + srcCount += isRMW ? BuildDelayFreeUses(intrin.op4, intrin.op1) : BuildOperandUses(intrin.op4); } } } From 1e76781dac2ed3e85db145884ed4b2aae87bc7ea Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 11 Jun 2021 16:40:09 -0700 Subject: [PATCH 5/6] remove TODO comment --- src/coreclr/jit/lsrabuild.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index f92ffa4b98b963..dcea8813119196 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3099,10 +3099,8 @@ int LinearScan::BuildDelayFreeUses(GenTree* node, GenTree* rmwNode, regMaskTP ca { // If node != rmwNode, then definitely node should be marked as "delayFree". // However, if node == rmwNode, then we can mark node as "delayFree" only - // none of the node/rmwNode are the last use. If either of them are last use, + // none of the node/rmwNode are the last uses. If either of them are last use, // we can safely reuse the rmwNode as destination. - // TODO: What happens to constants? Do they have same interval? - // TODO: Do we need `!use->lastUse`? if ((use->getInterval() != rmwInterval) || (!rmwIsLastUse && !use->lastUse)) { setDelayFree(use); From cae403836ea35eb6b1a665d7f60abe57dfaa2550 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 14 Jun 2021 10:33:55 -0700 Subject: [PATCH 6/6] review feedback --- src/coreclr/jit/lsraarm64.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index d6349054953d51..a7a14fdfe54f96 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -1117,22 +1117,6 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) bool forceOp2DelayFree = false; if ((intrin.id == NI_Vector64_GetElement) || (intrin.id == NI_Vector128_GetElement)) { - -#ifdef DEBUG - if (isRMW && intrin.op1->OperIs(GT_LCL_VAR)) - { - unsigned int varNum1 = intrin.op1->AsLclVar()->GetLclNum(); - bool op1LastUse = false; - - unsigned int varNum2 = BAD_VAR_NUM; - if (intrin.op2->OperIs(GT_LCL_VAR)) - { - varNum2 = intrin.op2->AsLclVar()->GetLclNum(); - assert((varNum1 == varNum2) && intrin.op2->HasLastUse()); - } - } -#endif - if (!intrin.op2->IsCnsIntOrI() && (!intrin.op1->isContained() || intrin.op1->OperIsLocal())) { // If the index is not a constant and the object is not contained or is a local