From a5a54c90d1b2a958f4f0cf4e29c3af357c1cb970 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 16 Sep 2024 16:23:09 -0700 Subject: [PATCH 1/7] Have comments and logging --- src/coreclr/jit/lsra.cpp | 127 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index dd766c51e3da73..cf3cd334b067f0 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -9714,6 +9714,19 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, } } + // kpathak + bool revert = false; + bool startLogging = false; + if ((fromBlock != nullptr) && (toBlock != nullptr)) + { + if ((fromBlock->bbNum == 17) && (toBlock->bbNum == 18)) + { + revert = true; + startLogging = true; + } + } + revert= true; + // First: // - Perform all moves from reg to stack (no ordering needed on these) // - For reg to reg moves, record the current location, associating their @@ -9776,6 +9789,15 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, source[toReg] = (regNumberSmall)fromReg; sourceIntervals[fromReg] = interval; targetRegsToDo |= genRegMask(toReg); + if (startLogging && genIsValidFloatReg(fromReg)) + { + printf("Added "); + interval->tinyDump(); + printf(" for %s -> %s (%s). targetRegsToDo= ", getRegName(fromReg), getRegName(toReg), + ((interval->registerType == TYP_DOUBLE) ? "double" : "float")); + compiler->dumpRegMask(targetRegsToDo.getLow(), TYP_FLOAT); + printf("\n"); + } } } @@ -9806,6 +9828,13 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, otherTargetInterval = sourceIntervals[otherHalfTargetReg]; } + //if (startLogging && genIsValidFloatReg(sourceReg)) + //{ + // printf("For "); + // interval->tinyDump(); + // printf(" from %s -> %s.\n", getRegName(sourceReg), getRegName(targetReg)); + //} + if (interval->registerType == TYP_DOUBLE) { // Condition 1 above. @@ -9817,7 +9846,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, targetRegsReady.AddRegNumInMask(targetReg); } } - else if ((otherTargetInterval != nullptr) && (otherTargetInterval->registerType == TYP_DOUBLE)) + else if (!revert && (otherTargetInterval != nullptr) && (otherTargetInterval->registerType == TYP_DOUBLE)) { // Condition 2 above. assert(otherHalfTargetReg != REG_NA); @@ -9839,10 +9868,28 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, // Perform reg to reg moves while (targetRegsToDo.IsNonEmpty()) { + if (startLogging) + { + printf("targetRegsReady: "); + compiler->dumpRegMask(targetRegsReady.getLow(), TYP_FLOAT); + printf(", targetRegsToDo: "); + compiler->dumpRegMask(targetRegsToDo.getLow(), TYP_FLOAT); + printf("\n"); + } while (targetRegsReady.IsNonEmpty()) { regNumber targetReg = genFirstRegNumFromMaskAndToggle(targetRegsReady); targetRegsToDo.RemoveRegNumFromMask(targetReg); + if (startLogging) + { + printf("targetReg: %s, ", getRegName(targetReg)); + printf("targetRegsReady: "); + compiler->dumpRegMask(targetRegsReady.getLow(), TYP_FLOAT); + printf(", targetRegsToDo: "); + compiler->dumpRegMask(targetRegsToDo.getLow(), TYP_FLOAT); + printf("\n"); + } + assert(location[targetReg] != targetReg); assert(targetReg < REG_COUNT); regNumber sourceReg = (regNumber)source[targetReg]; @@ -9854,6 +9901,15 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, assert(interval != nullptr); addResolution(block, insertionPoint, interval, targetReg, fromReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); + + if (startLogging && genIsValidFloatReg(sourceReg)) + { + printf("***** Added resolution: %s -> %s (%s)\n", getRegName(fromReg), getRegName(targetReg), (interval->registerType == TYP_DOUBLE) ? "double" : "float"); + printf(" Removing "); + interval->tinyDump(); + printf("for %s.\n", getRegName(sourceReg)); + } + sourceIntervals[sourceReg] = nullptr; location[sourceReg] = REG_NA; @@ -9878,7 +9934,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, // Since we want to check if we can free upperHalfReg, only do it // if lowerHalfReg is ready. - if (targetRegsReady.IsRegNumInMask(fromReg)) + if (!revert && targetRegsReady.IsRegNumInMask(fromReg)) { regNumber upperHalfSrcReg = (regNumber)source[upperHalfReg]; regNumber upperHalfSrcLoc = (regNumber)location[upperHalfReg]; @@ -10020,6 +10076,13 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); location[source[otherTargetReg]] = REG_STK; + if (startLogging && genIsValidFloatReg(sourceReg)) + { + printf("***** Added resolution: %s -> %s (%s)\n", getRegName(targetReg), getRegName(REG_STK), (otherInterval->registerType == TYP_DOUBLE) ? "double" : "float"); + printf(" Removing "); + sourceIntervals[targetReg]->tinyDump(); + printf("for %s.\n", getRegName(sourceReg)); + } regMaskTP otherTargetRegMask = genRegMask(otherTargetReg); targetRegsFromStack |= otherTargetRegMask; @@ -10030,6 +10093,14 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolution(block, insertionPoint, sourceIntervals[sourceReg], targetReg, fromReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); + if (startLogging && genIsValidFloatReg(sourceReg)) + { + printf("***** Added resolution: %s -> %s\n", getRegName(fromReg), getRegName(targetReg), + (sourceIntervals[sourceReg]->registerType == TYP_DOUBLE) ? "double" : "float"); + printf(" Removing "); + sourceIntervals[sourceReg]->tinyDump(); + printf("for %s.\n", getRegName(sourceReg)); + } location[sourceReg] = REG_NA; // Add its "fromReg" to "targetRegsReady", only if: @@ -10076,22 +10147,58 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, targetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); + if (startLogging && genIsValidFloatReg(targetReg)) + { + printf("***** Added resolution: %s -> %s (double)\n", getRegName(targetReg), getRegName(tempReg)); + printf(" Removing "); + sourceIntervals[targetReg]->tinyDump(); + printf("for %s.\n", getRegName(targetReg)); + } } - else if (otherTargetInterval != nullptr) + else if (!revert && otherTargetInterval != nullptr) { assert(otherHalfTargetReg != REG_NA); assert(otherTargetInterval->registerType == TYP_DOUBLE); addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, otherHalfTargetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); + if (startLogging && genIsValidFloatReg(otherHalfTargetReg)) + { + printf("***** Added resolution: %s -> %s (double)\n", getRegName(otherHalfTargetReg), + getRegName(tempReg)); + printf(" Removing "); + sourceIntervals[otherHalfTargetReg]->tinyDump(); + printf("for %s.\n", getRegName(otherHalfTargetReg)); + } } else { assert(sourceIntervals[targetReg] != nullptr); - addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, - targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) - DEBUG_ARG(resolveTypeName[resolveType])); + if (!revert && sourceIntervals[targetReg]->registerType == TYP_DOUBLE) + { + addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, + targetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); + regNumber upperHalfReg = REG_NEXT(targetReg); + if (targetRegsToDo.IsRegNumInMask(upperHalfReg)) + { + targetRegsReady |= genRegMask(upperHalfReg); + } + } + else + { + addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, + targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) + DEBUG_ARG(resolveTypeName[resolveType])); + } + if (startLogging && genIsValidFloatReg(targetReg)) + { + printf("***** Added resolution: %s -> %s (%s)\n", getRegName(targetReg), getRegName(tempReg), + (sourceIntervals[targetReg]->registerType == TYP_DOUBLE) ? "double" : "float"); + printf(" Removing "); + sourceIntervals[targetReg]->tinyDump(); + printf("for %s.\n", getRegName(targetReg)); + } location[targetReg] = (regNumberSmall)tempReg; } #else @@ -10119,6 +10226,14 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolution(block, insertionPoint, interval, targetReg, REG_STK DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); + if (startLogging && genIsValidFloatReg(targetReg)) + { + printf("***** Added resolution: %s -> %s (%s)\n", getRegName(REG_STK), getRegName(targetReg), + (interval->registerType == TYP_DOUBLE) ? "double" : "float"); + printf(" Removing "); + interval->tinyDump(); + printf("for %s.\n", getRegName(REG_STK)); + } } } From 7ce5caf437a87480b1106f3140c6114caa901297 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 16 Sep 2024 16:30:16 -0700 Subject: [PATCH 2/7] Add JITDUMP for resolutions --- src/coreclr/jit/lsra.cpp | 95 +++++++++------------------------------- 1 file changed, 20 insertions(+), 75 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index cf3cd334b067f0..7dbddd54a60277 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -9714,18 +9714,18 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, } } - // kpathak - bool revert = false; - bool startLogging = false; - if ((fromBlock != nullptr) && (toBlock != nullptr)) + if (compiler->verbose) { - if ((fromBlock->bbNum == 17) && (toBlock->bbNum == 18)) + printf("Resolving Edge: "); + if (fromBlock != nullptr) { - revert = true; - startLogging = true; + printf("from: " FMT_BB " ", fromBlock->bbNum); + } + if (toBlock != nullptr) + { + printf("to: " FMT_BB, toBlock->bbNum); } } - revert= true; // First: // - Perform all moves from reg to stack (no ordering needed on these) @@ -9789,7 +9789,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, source[toReg] = (regNumberSmall)fromReg; sourceIntervals[fromReg] = interval; targetRegsToDo |= genRegMask(toReg); - if (startLogging && genIsValidFloatReg(fromReg)) + if (compiler->verbose) { printf("Added "); interval->tinyDump(); @@ -9846,7 +9846,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, targetRegsReady.AddRegNumInMask(targetReg); } } - else if (!revert && (otherTargetInterval != nullptr) && (otherTargetInterval->registerType == TYP_DOUBLE)) + else if ((otherTargetInterval != nullptr) && (otherTargetInterval->registerType == TYP_DOUBLE)) { // Condition 2 above. assert(otherHalfTargetReg != REG_NA); @@ -9868,25 +9868,23 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, // Perform reg to reg moves while (targetRegsToDo.IsNonEmpty()) { - if (startLogging) + if (compiler->verbose) { - printf("targetRegsReady: "); - compiler->dumpRegMask(targetRegsReady.getLow(), TYP_FLOAT); - printf(", targetRegsToDo: "); - compiler->dumpRegMask(targetRegsToDo.getLow(), TYP_FLOAT); + printf("targetRegsToDo: "); + compiler->dumpRegMask(targetRegsToDo); printf("\n"); } while (targetRegsReady.IsNonEmpty()) { regNumber targetReg = genFirstRegNumFromMaskAndToggle(targetRegsReady); targetRegsToDo.RemoveRegNumFromMask(targetReg); - if (startLogging) + if (compiler->verbose) { printf("targetReg: %s, ", getRegName(targetReg)); printf("targetRegsReady: "); - compiler->dumpRegMask(targetRegsReady.getLow(), TYP_FLOAT); + compiler->dumpRegMask(targetRegsReady); printf(", targetRegsToDo: "); - compiler->dumpRegMask(targetRegsToDo.getLow(), TYP_FLOAT); + compiler->dumpRegMask(targetRegsToDo); printf("\n"); } @@ -9902,14 +9900,6 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolution(block, insertionPoint, interval, targetReg, fromReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); - if (startLogging && genIsValidFloatReg(sourceReg)) - { - printf("***** Added resolution: %s -> %s (%s)\n", getRegName(fromReg), getRegName(targetReg), (interval->registerType == TYP_DOUBLE) ? "double" : "float"); - printf(" Removing "); - interval->tinyDump(); - printf("for %s.\n", getRegName(sourceReg)); - } - sourceIntervals[sourceReg] = nullptr; location[sourceReg] = REG_NA; @@ -9934,7 +9924,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, // Since we want to check if we can free upperHalfReg, only do it // if lowerHalfReg is ready. - if (!revert && targetRegsReady.IsRegNumInMask(fromReg)) + if (targetRegsReady.IsRegNumInMask(fromReg)) { regNumber upperHalfSrcReg = (regNumber)source[upperHalfReg]; regNumber upperHalfSrcLoc = (regNumber)location[upperHalfReg]; @@ -10076,13 +10066,6 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); location[source[otherTargetReg]] = REG_STK; - if (startLogging && genIsValidFloatReg(sourceReg)) - { - printf("***** Added resolution: %s -> %s (%s)\n", getRegName(targetReg), getRegName(REG_STK), (otherInterval->registerType == TYP_DOUBLE) ? "double" : "float"); - printf(" Removing "); - sourceIntervals[targetReg]->tinyDump(); - printf("for %s.\n", getRegName(sourceReg)); - } regMaskTP otherTargetRegMask = genRegMask(otherTargetReg); targetRegsFromStack |= otherTargetRegMask; @@ -10093,14 +10076,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolution(block, insertionPoint, sourceIntervals[sourceReg], targetReg, fromReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); - if (startLogging && genIsValidFloatReg(sourceReg)) - { - printf("***** Added resolution: %s -> %s\n", getRegName(fromReg), getRegName(targetReg), - (sourceIntervals[sourceReg]->registerType == TYP_DOUBLE) ? "double" : "float"); - printf(" Removing "); - sourceIntervals[sourceReg]->tinyDump(); - printf("for %s.\n", getRegName(sourceReg)); - } + location[sourceReg] = REG_NA; // Add its "fromReg" to "targetRegsReady", only if: @@ -10147,35 +10123,20 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, targetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); - if (startLogging && genIsValidFloatReg(targetReg)) - { - printf("***** Added resolution: %s -> %s (double)\n", getRegName(targetReg), getRegName(tempReg)); - printf(" Removing "); - sourceIntervals[targetReg]->tinyDump(); - printf("for %s.\n", getRegName(targetReg)); - } } - else if (!revert && otherTargetInterval != nullptr) + else if (otherTargetInterval != nullptr) { assert(otherHalfTargetReg != REG_NA); assert(otherTargetInterval->registerType == TYP_DOUBLE); addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, otherHalfTargetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); - if (startLogging && genIsValidFloatReg(otherHalfTargetReg)) - { - printf("***** Added resolution: %s -> %s (double)\n", getRegName(otherHalfTargetReg), - getRegName(tempReg)); - printf(" Removing "); - sourceIntervals[otherHalfTargetReg]->tinyDump(); - printf("for %s.\n", getRegName(otherHalfTargetReg)); - } } else { assert(sourceIntervals[targetReg] != nullptr); - if (!revert && sourceIntervals[targetReg]->registerType == TYP_DOUBLE) + if (sourceIntervals[targetReg]->registerType == TYP_DOUBLE) { addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, targetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); @@ -10191,14 +10152,6 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); } - if (startLogging && genIsValidFloatReg(targetReg)) - { - printf("***** Added resolution: %s -> %s (%s)\n", getRegName(targetReg), getRegName(tempReg), - (sourceIntervals[targetReg]->registerType == TYP_DOUBLE) ? "double" : "float"); - printf(" Removing "); - sourceIntervals[targetReg]->tinyDump(); - printf("for %s.\n", getRegName(targetReg)); - } location[targetReg] = (regNumberSmall)tempReg; } #else @@ -10226,14 +10179,6 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolution(block, insertionPoint, interval, targetReg, REG_STK DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); - if (startLogging && genIsValidFloatReg(targetReg)) - { - printf("***** Added resolution: %s -> %s (%s)\n", getRegName(REG_STK), getRegName(targetReg), - (interval->registerType == TYP_DOUBLE) ? "double" : "float"); - printf(" Removing "); - interval->tinyDump(); - printf("for %s.\n", getRegName(REG_STK)); - } } } From beb2d0ed4096423b0f4fdac153232a2fafc2456d Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 16 Sep 2024 16:48:34 -0700 Subject: [PATCH 3/7] cleanup --- src/coreclr/jit/lsra.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 7dbddd54a60277..0a571ecb50693c 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -10076,7 +10076,6 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolution(block, insertionPoint, sourceIntervals[sourceReg], targetReg, fromReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); - location[sourceReg] = REG_NA; // Add its "fromReg" to "targetRegsReady", only if: @@ -10136,22 +10135,22 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, { assert(sourceIntervals[targetReg] != nullptr); + addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, + targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) + DEBUG_ARG(resolveTypeName[resolveType])); + if (sourceIntervals[targetReg]->registerType == TYP_DOUBLE) { - addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, - targetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); + // If targetReg that got freed-up held DOUBLE value, then + // free up the upper half register as well, if it is a + // target of some other interval. regNumber upperHalfReg = REG_NEXT(targetReg); if (targetRegsToDo.IsRegNumInMask(upperHalfReg)) { targetRegsReady |= genRegMask(upperHalfReg); } } - else - { - addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, - targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) - DEBUG_ARG(resolveTypeName[resolveType])); - } + location[targetReg] = (regNumberSmall)tempReg; } #else From 477d0df2ac964c528270491046e2124725ffd210 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 16 Sep 2024 17:09:48 -0700 Subject: [PATCH 4/7] handle for other cases too --- src/coreclr/jit/lsra.cpp | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 0a571ecb50693c..fc4efb770ba331 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -10108,6 +10108,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, { compiler->codeGen->regSet.rsSetRegsModified(genRegMask(tempReg) DEBUGARG(true)); #ifdef TARGET_ARM + regNumber originDoubleReg = REG_NA; Interval* otherTargetInterval = nullptr; regNumber otherHalfTargetReg = REG_NA; if (genIsValidFloatReg(targetReg) && !genIsValidDoubleReg(targetReg)) @@ -10122,6 +10123,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, targetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); + originDoubleReg = targetReg; } else if (otherTargetInterval != nullptr) { @@ -10130,6 +10132,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, addResolutionForDouble(block, insertionPoint, sourceIntervals, location, tempReg, otherHalfTargetReg, resolveType DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock)); + originDoubleReg = otherHalfTargetReg; } else { @@ -10141,14 +10144,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, if (sourceIntervals[targetReg]->registerType == TYP_DOUBLE) { - // If targetReg that got freed-up held DOUBLE value, then - // free up the upper half register as well, if it is a - // target of some other interval. - regNumber upperHalfReg = REG_NEXT(targetReg); - if (targetRegsToDo.IsRegNumInMask(upperHalfReg)) - { - targetRegsReady |= genRegMask(upperHalfReg); - } + originDoubleReg = targetReg; } location[targetReg] = (regNumberSmall)tempReg; @@ -10160,6 +10156,21 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); location[targetReg] = (regNumberSmall)tempReg; + + if (originDoubleReg != REG_NA) + { + // There was a value in originDoubleReg, which we free-up by moving it to + // tempReg. As such, make sure to free-up the upper-half too, only if + // originDoubleReg held DOUBLE interval. + assert(genIsValidDoubleReg(originDoubleReg)); + targetRegMask |= genRegMask(originDoubleReg); + + regNumber upperHalfReg = REG_NEXT(originDoubleReg); + if (targetRegsToDo.IsRegNumInMask(upperHalfReg)) + { + targetRegMask |= genRegMask(upperHalfReg); + } + } #endif // TARGET_ARM targetRegsReady |= targetRegMask; } From 293411c4995f9a0461acb042f46ece6822d0e166 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 16 Sep 2024 17:12:14 -0700 Subject: [PATCH 5/7] jit format --- src/coreclr/jit/lsra.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index fc4efb770ba331..d13a8480b504bf 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -9828,12 +9828,12 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, otherTargetInterval = sourceIntervals[otherHalfTargetReg]; } - //if (startLogging && genIsValidFloatReg(sourceReg)) + // if (startLogging && genIsValidFloatReg(sourceReg)) //{ - // printf("For "); - // interval->tinyDump(); - // printf(" from %s -> %s.\n", getRegName(sourceReg), getRegName(targetReg)); - //} + // printf("For "); + // interval->tinyDump(); + // printf(" from %s -> %s.\n", getRegName(sourceReg), getRegName(targetReg)); + // } if (interval->registerType == TYP_DOUBLE) { @@ -10108,7 +10108,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, { compiler->codeGen->regSet.rsSetRegsModified(genRegMask(tempReg) DEBUGARG(true)); #ifdef TARGET_ARM - regNumber originDoubleReg = REG_NA; + regNumber originDoubleReg = REG_NA; Interval* otherTargetInterval = nullptr; regNumber otherHalfTargetReg = REG_NA; if (genIsValidFloatReg(targetReg) && !genIsValidDoubleReg(targetReg)) @@ -10138,13 +10138,13 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, { assert(sourceIntervals[targetReg] != nullptr); - addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, + addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) DEBUG_ARG(resolveTypeName[resolveType])); if (sourceIntervals[targetReg]->registerType == TYP_DOUBLE) { - originDoubleReg = targetReg; + originDoubleReg = targetReg; } location[targetReg] = (regNumberSmall)tempReg; From c75740d0287e576e0436383b063ec721c4efbb8e Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 16 Sep 2024 18:01:29 -0700 Subject: [PATCH 6/7] minor fix --- src/coreclr/jit/lsra.cpp | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index d13a8480b504bf..635aaf330e5fa4 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -9828,13 +9828,6 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, otherTargetInterval = sourceIntervals[otherHalfTargetReg]; } - // if (startLogging && genIsValidFloatReg(sourceReg)) - //{ - // printf("For "); - // interval->tinyDump(); - // printf(" from %s -> %s.\n", getRegName(sourceReg), getRegName(targetReg)); - // } - if (interval->registerType == TYP_DOUBLE) { // Condition 1 above. @@ -10149,19 +10142,13 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, location[targetReg] = (regNumberSmall)tempReg; } -#else - assert(sourceIntervals[targetReg] != nullptr); - - addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, - targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) - DEBUG_ARG(resolveTypeName[resolveType])); - location[targetReg] = (regNumberSmall)tempReg; if (originDoubleReg != REG_NA) { // There was a value in originDoubleReg, which we free-up by moving it to // tempReg. As such, make sure to free-up the upper-half too, only if - // originDoubleReg held DOUBLE interval. + // originDoubleReg held DOUBLE interval and upper-half is target of a + // different interval. assert(genIsValidDoubleReg(originDoubleReg)); targetRegMask |= genRegMask(originDoubleReg); @@ -10171,6 +10158,13 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, targetRegMask |= genRegMask(upperHalfReg); } } +#else + assert(sourceIntervals[targetReg] != nullptr); + + addResolution(block, insertionPoint, sourceIntervals[targetReg], tempReg, + targetReg DEBUG_ARG(fromBlock) DEBUG_ARG(toBlock) + DEBUG_ARG(resolveTypeName[resolveType])); + location[targetReg] = (regNumberSmall)tempReg; #endif // TARGET_ARM targetRegsReady |= targetRegMask; } From 51206d7d0f6f6b94b66783c99a5598dde6a40f69 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 16 Sep 2024 21:22:05 -0700 Subject: [PATCH 7/7] fix build errors --- src/coreclr/jit/lsra.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 635aaf330e5fa4..e63cc68361a1a6 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -9714,6 +9714,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, } } +#ifdef DEBUG if (compiler->verbose) { printf("Resolving Edge: "); @@ -9726,6 +9727,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, printf("to: " FMT_BB, toBlock->bbNum); } } +#endif // DEBUG // First: // - Perform all moves from reg to stack (no ordering needed on these) @@ -9789,6 +9791,8 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, source[toReg] = (regNumberSmall)fromReg; sourceIntervals[fromReg] = interval; targetRegsToDo |= genRegMask(toReg); + +#ifdef DEBUG if (compiler->verbose) { printf("Added "); @@ -9798,6 +9802,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, compiler->dumpRegMask(targetRegsToDo.getLow(), TYP_FLOAT); printf("\n"); } +#endif // DEBUG } } @@ -9861,16 +9866,19 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, // Perform reg to reg moves while (targetRegsToDo.IsNonEmpty()) { +#ifdef DEBUG if (compiler->verbose) { printf("targetRegsToDo: "); compiler->dumpRegMask(targetRegsToDo); printf("\n"); } +#endif // DEBUG while (targetRegsReady.IsNonEmpty()) { regNumber targetReg = genFirstRegNumFromMaskAndToggle(targetRegsReady); targetRegsToDo.RemoveRegNumFromMask(targetReg); +#ifdef DEBUG if (compiler->verbose) { printf("targetReg: %s, ", getRegName(targetReg)); @@ -9880,6 +9888,7 @@ void LinearScan::resolveEdge(BasicBlock* fromBlock, compiler->dumpRegMask(targetRegsToDo); printf("\n"); } +#endif // DEBUG assert(location[targetReg] != targetReg); assert(targetReg < REG_COUNT);