From e16cf49479c100d3c407884f567883b57dd16b01 Mon Sep 17 00:00:00 2001 From: Pat Gavlin Date: Wed, 28 Jun 2017 11:39:50 -0700 Subject: [PATCH] Refactor LSRA's operand use builder. Commit migrated from https://github.com/dotnet/coreclr/commit/5fcc39e514645dd7c33b327250e7b1c8c86814f4 --- src/coreclr/src/jit/lsra.cpp | 256 ++++++++++++++++++++----------------------- 1 file changed, 120 insertions(+), 136 deletions(-) diff --git a/src/coreclr/src/jit/lsra.cpp b/src/coreclr/src/jit/lsra.cpp index 2af8251..fbcaa88 100644 --- a/src/coreclr/src/jit/lsra.cpp +++ b/src/coreclr/src/jit/lsra.cpp @@ -3921,168 +3921,152 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, int internalCount = buildInternalRegisterDefsForNode(tree, currentLoc, internalRefs DEBUG_ARG(minRegCount)); // pop all ref'd tree temps - GenTreeOperandIterator iterator = tree->OperandsBegin(); - - // `operandDefs` holds the list of `LocationInfo` values for the registers defined by the current - // operand. `operandDefsIterator` points to the current `LocationInfo` value in `operandDefs`. - LocationInfoList operandDefs; - LocationInfoListNode* operandDefsIterator = operandDefs.End(); - for (int useIndex = 0; useIndex < consume; useIndex++) + LocationInfoList operandDefs; + for (GenTree* operand : tree->Operands()) { - // If we've consumed all of the registers defined by the current operand, advance to the next - // operand that defines any registers. - if (operandDefsIterator == operandDefs.End()) + // Skip operands that do not define any registers, whether directly or indirectly. + if (!operand->gtLsraInfo.definesAnyRegisters) { - // Skip operands that do not define any registers, whether directly or indirectly. - GenTree* operand; - do - { - assert(iterator != tree->OperandsEnd()); - operand = *iterator; - - ++iterator; - } while (!operand->gtLsraInfo.definesAnyRegisters); - - // If we have already processed a previous operand, return its `LocationInfo` list to the - // pool. - if (useIndex > 0) - { - assert(!operandDefs.IsEmpty()); - listNodePool.ReturnNodes(operandDefs); - } - - // Remove the list of registers defined by the current operand from the map. Note that this - // is only correct because tree nodes are singly-used: if this property ever changes (e.g. - // if tree nodes are eventually allowed to be multiply-used), then the removal is only - // correct at the last use. - bool removed = operandToLocationInfoMap.TryRemove(operand, &operandDefs); - assert(removed); + continue; + } - // Move the operand def iterator to the `LocationInfo` for the first register defined by the - // current operand. - operandDefsIterator = operandDefs.Begin(); - assert(operandDefsIterator != operandDefs.End()); + // If we have already processed a previous operand, return its `LocationInfo` list to the + // pool. + if (!operandDefs.IsEmpty()) + { + assert(!operandDefs.IsEmpty()); + listNodePool.ReturnNodes(operandDefs); } - LocationInfo& locInfo = *static_cast(operandDefsIterator); - operandDefsIterator = operandDefsIterator->Next(); + // Remove the list of registers defined by the current operand from the map. Note that this + // is only correct because tree nodes are singly-used: if this property ever changes (e.g. + // if tree nodes are eventually allowed to be multiply-used), then the removal is only + // correct at the last use. + bool removed = operandToLocationInfoMap.TryRemove(operand, &operandDefs); + assert(removed); - JITDUMP("t%u ", locInfo.loc); + LocationInfoListNode* const operandDefsEnd = operandDefs.End(); + for (LocationInfoListNode* operandDefsIterator = operandDefs.Begin(); operandDefsIterator != operandDefsEnd; operandDefsIterator = operandDefsIterator->Next()) + { + LocationInfo& locInfo = *static_cast(operandDefsIterator); - // for interstitial tree temps, a use is always last and end; - // this is set by default in newRefPosition - GenTree* useNode = locInfo.treeNode; - assert(useNode != nullptr); - var_types type = useNode->TypeGet(); - regMaskTP candidates = getUseCandidates(useNode); - Interval* i = locInfo.interval; - unsigned multiRegIdx = locInfo.multiRegIdx; + JITDUMP("t%u ", locInfo.loc); + + // for interstitial tree temps, a use is always last and end; + // this is set by default in newRefPosition + GenTree* useNode = locInfo.treeNode; + assert(useNode != nullptr); + var_types type = useNode->TypeGet(); + regMaskTP candidates = getUseCandidates(useNode); + Interval* i = locInfo.interval; + unsigned multiRegIdx = locInfo.multiRegIdx; #ifdef FEATURE_SIMD - // In case of multi-reg call store to a local, there won't be any mismatch of - // use candidates with the type of the tree node. - if (tree->OperIsLocalStore() && varDefInterval == nullptr && !useNode->IsMultiRegCall()) - { - // This is a non-candidate store. If this is a SIMD type, the use candidates - // may not match the type of the tree node. If that is the case, change the - // type of the tree node to match, so that we do the right kind of store. - if ((candidates & allRegs(tree->gtType)) == RBM_NONE) + // In case of multi-reg call store to a local, there won't be any mismatch of + // use candidates with the type of the tree node. + if (tree->OperIsLocalStore() && varDefInterval == nullptr && !useNode->IsMultiRegCall()) { - noway_assert((candidates & allRegs(useNode->gtType)) != RBM_NONE); - // Currently, the only case where this should happen is for a TYP_LONG - // source and a TYP_SIMD8 target. - assert((useNode->gtType == TYP_LONG && tree->gtType == TYP_SIMD8) || - (useNode->gtType == TYP_SIMD8 && tree->gtType == TYP_LONG)); - tree->gtType = useNode->gtType; + // This is a non-candidate store. If this is a SIMD type, the use candidates + // may not match the type of the tree node. If that is the case, change the + // type of the tree node to match, so that we do the right kind of store. + if ((candidates & allRegs(tree->gtType)) == RBM_NONE) + { + noway_assert((candidates & allRegs(useNode->gtType)) != RBM_NONE); + // Currently, the only case where this should happen is for a TYP_LONG + // source and a TYP_SIMD8 target. + assert((useNode->gtType == TYP_LONG && tree->gtType == TYP_SIMD8) || + (useNode->gtType == TYP_SIMD8 && tree->gtType == TYP_LONG)); + tree->gtType = useNode->gtType; + } } - } #endif // FEATURE_SIMD - if (useNode->gtLsraInfo.isTgtPref) - { - prefSrcInterval = i; - } + if (useNode->gtLsraInfo.isTgtPref) + { + prefSrcInterval = i; + } - regMaskTP fixedAssignment = fixedCandidateMask(type, candidates); - if (fixedAssignment != RBM_NONE) - { - candidates = fixedAssignment; - } + regMaskTP fixedAssignment = fixedCandidateMask(type, candidates); + if (fixedAssignment != RBM_NONE) + { + candidates = fixedAssignment; + } - const bool regOptionalAtUse = useNode->IsRegOptional(); - const bool delayRegFree = (hasDelayFreeSrc && useNode->gtLsraInfo.isDelayFree); + const bool regOptionalAtUse = useNode->IsRegOptional(); + const bool delayRegFree = (hasDelayFreeSrc && useNode->gtLsraInfo.isDelayFree); - assert(isCandidateLocalRef(useNode) == i->isLocalVar); - if (!i->isLocalVar) - { - // For non-localVar uses we record nothing, - // as nothing needs to be written back to the tree. - useNode = nullptr; - } + assert(isCandidateLocalRef(useNode) == i->isLocalVar); + if (!i->isLocalVar) + { + // For non-localVar uses we record nothing, + // as nothing needs to be written back to the tree. + useNode = nullptr; + } #ifdef DEBUG - // If delayRegFree, then Use will interfere with the destination of - // the consuming node. Therefore, we also need add the kill set of - // consuming node to minRegCount. - // - // For example consider the following IR on x86, where v01 and v02 - // are method args coming in ecx and edx respectively. - // GT_DIV(v01, v02) - // - // For GT_DIV minRegCount will be 3 without adding kill set - // of GT_DIV node. - // - // Assume further JitStressRegs=2, which would constrain - // candidates to callee trashable regs { eax, ecx, edx } on - // use positions of v01 and v02. LSRA allocates ecx for v01. - // Use position of v02 cannot be allocated a regs since it - // is marked delay-reg free and {eax,edx} are getting killed - // before the def of GT_DIV. For this reason, minRegCount - // for Use position of v02 also needs to take into account - // of kill set of its consuming node. - unsigned minRegCountForUsePos = minRegCount; - if (delayRegFree) - { - regMaskTP killMask = getKillSetForNode(tree); - if (killMask != RBM_NONE) + // If delayRegFree, then Use will interfere with the destination of + // the consuming node. Therefore, we also need add the kill set of + // consuming node to minRegCount. + // + // For example consider the following IR on x86, where v01 and v02 + // are method args coming in ecx and edx respectively. + // GT_DIV(v01, v02) + // + // For GT_DIV minRegCount will be 3 without adding kill set + // of GT_DIV node. + // + // Assume further JitStressRegs=2, which would constrain + // candidates to callee trashable regs { eax, ecx, edx } on + // use positions of v01 and v02. LSRA allocates ecx for v01. + // Use position of v02 cannot be allocated a regs since it + // is marked delay-reg free and {eax,edx} are getting killed + // before the def of GT_DIV. For this reason, minRegCount + // for Use position of v02 also needs to take into account + // of kill set of its consuming node. + unsigned minRegCountForUsePos = minRegCount; + if (delayRegFree) { - minRegCountForUsePos += genCountBits(killMask); + regMaskTP killMask = getKillSetForNode(tree); + if (killMask != RBM_NONE) + { + minRegCountForUsePos += genCountBits(killMask); + } } - } #endif // DEBUG - RefPosition* pos; - if ((candidates & allRegs(i->registerType)) == 0) - { - // This should only occur where we've got a type mismatch due to SIMD - // pointer-size types that are passed & returned as longs. - i->hasConflictingDefUse = true; - if (fixedAssignment != RBM_NONE) + RefPosition* pos; + if ((candidates & allRegs(i->registerType)) == 0) { - // Explicitly insert a FixedRefPosition and fake the candidates, because otherwise newRefPosition - // will complain about the types not matching. - regNumber physicalReg = genRegNumFromMask(fixedAssignment); - RefPosition* pos = newRefPosition(physicalReg, currentLoc, RefTypeFixedReg, nullptr, fixedAssignment); + // This should only occur where we've got a type mismatch due to SIMD + // pointer-size types that are passed & returned as longs. + i->hasConflictingDefUse = true; + if (fixedAssignment != RBM_NONE) + { + // Explicitly insert a FixedRefPosition and fake the candidates, because otherwise newRefPosition + // will complain about the types not matching. + regNumber physicalReg = genRegNumFromMask(fixedAssignment); + RefPosition* pos = newRefPosition(physicalReg, currentLoc, RefTypeFixedReg, nullptr, fixedAssignment); + } + pos = newRefPosition(i, currentLoc, RefTypeUse, useNode, allRegs(i->registerType), + multiRegIdx DEBUG_ARG(minRegCountForUsePos)); + pos->registerAssignment = candidates; + } + else + { + pos = newRefPosition(i, currentLoc, RefTypeUse, useNode, candidates, + multiRegIdx DEBUG_ARG(minRegCountForUsePos)); } - pos = newRefPosition(i, currentLoc, RefTypeUse, useNode, allRegs(i->registerType), - multiRegIdx DEBUG_ARG(minRegCountForUsePos)); - pos->registerAssignment = candidates; - } - else - { - pos = newRefPosition(i, currentLoc, RefTypeUse, useNode, candidates, - multiRegIdx DEBUG_ARG(minRegCountForUsePos)); - } - if (delayRegFree) - { - hasDelayFreeSrc = true; - pos->delayRegFree = true; - } + if (delayRegFree) + { + hasDelayFreeSrc = true; + pos->delayRegFree = true; + } - if (regOptionalAtUse) - { - pos->setAllocateIfProfitable(true); + if (regOptionalAtUse) + { + pos->setAllocateIfProfitable(true); + } } } JITDUMP("\n"); -- 2.7.4