From b194eb0af9e373c7fd9021ead8343abc1f876740 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 16 Feb 2021 23:01:23 -0800 Subject: [PATCH 1/3] Fix majority of libraries-jitstressregs When we verify final allocation, we were not resetting the `assignedInterval` if the interval got assigned to a different register. This was not consistent to what we do during allocation. Fixed following failures: https://dev.azure.com/dnceng/public/_build/results?buildId=989595&view=ms.vss-test-web.build-test-results-tab&runId=30998546&resultId=169282&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-47307-head-5f85fedec0cd4ff9a0/System.Net.Security.Tests/console.73e69194.log?sv=2019-07-07&se=2021-03-03T08%3A52%3A21Z&sr=c&sp=rl&sig=naQOrM%2BszKDyDAO8m%2BU%2Fu149f4cjodTk1dYCKl9cvKs%3D Failures happen on arch/OS - Linux arm64 - Linux arm - Linux x64 - Windows x86 - Windows x64 --- src/coreclr/jit/lsra.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 996dcac0dd9f91..ac1788b846c40b 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -10773,7 +10773,7 @@ void LinearScan::verifyFinalAllocation() { // Free Registers. // We could use the freeRegisters() method, but we'd have to carefully manage the active intervals. - for (regNumber reg = REG_FIRST; reg < ACTUAL_REG_COUNT; reg = REG_NEXT(reg)) + for (regNumber reg = REG_FIRST; reg < ACTUAL_REG_COUNT && regsToFree != RBM_NONE; reg = REG_NEXT(reg)) { regMaskTP regMask = genRegMask(reg); if ((regsToFree & regMask) != RBM_NONE) @@ -10952,6 +10952,7 @@ void LinearScan::verifyFinalAllocation() else if (RefTypeIsDef(currentRefPosition->refType)) { interval->isActive = true; + if (VERBOSE) { if (interval->isConstant && (currentRefPosition->treeNode != nullptr) && @@ -11023,11 +11024,17 @@ void LinearScan::verifyFinalAllocation() } else { - if (!currentRefPosition->copyReg) + if (RefTypeIsDef(currentRefPosition->refType)) { - interval->physReg = regNum; - interval->assignedReg = regRecord; + // Interval was assigned to a different register. + // Clear the assigned interval of current register. + if (interval->physReg != REG_NA && interval->physReg != regNum) + { + interval->assignedReg->assignedInterval = nullptr; + } } + interval->physReg = regNum; + interval->assignedReg = regRecord; regRecord->assignedInterval = interval; } } From 6bac5b5a4a7b728edfbccb61663c602539fca2f5 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 26 Feb 2021 09:59:05 -0800 Subject: [PATCH 2/3] Delete regsToFree and delayRegsToFree from verifyFinalAllocation These two variables were always set to `RBM_NONE`. As part of https://github.com/dotnet/runtime/issues/48837, investigate how to reintroduce them. --- src/coreclr/jit/lsra.cpp | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index ac1788b846c40b..aa3ebb99105471 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -10723,8 +10723,6 @@ void LinearScan::verifyFinalAllocation() BasicBlock* currentBlock = nullptr; GenTree* firstBlockEndResolutionNode = nullptr; - regMaskTP regsToFree = RBM_NONE; - regMaskTP delayRegsToFree = RBM_NONE; LsraLocation currentLocation = MinLocation; for (RefPosition& refPosition : refPositions) { @@ -10734,12 +10732,7 @@ void LinearScan::verifyFinalAllocation() regNumber regNum = REG_NA; activeRefPosition = currentRefPosition; - if (currentRefPosition->refType == RefTypeBB) - { - regsToFree |= delayRegsToFree; - delayRegsToFree = RBM_NONE; - } - else + if (currentRefPosition->refType != RefTypeBB) { if (currentRefPosition->IsPhysRegRef()) { @@ -10768,23 +10761,6 @@ void LinearScan::verifyFinalAllocation() } LsraLocation newLocation = currentRefPosition->nodeLocation; - - if (newLocation > currentLocation) - { - // Free Registers. - // We could use the freeRegisters() method, but we'd have to carefully manage the active intervals. - for (regNumber reg = REG_FIRST; reg < ACTUAL_REG_COUNT && regsToFree != RBM_NONE; reg = REG_NEXT(reg)) - { - regMaskTP regMask = genRegMask(reg); - if ((regsToFree & regMask) != RBM_NONE) - { - RegRecord* physRegRecord = getRegisterRecord(reg); - physRegRecord->assignedInterval = nullptr; - } - } - regsToFree = delayRegsToFree; - regsToFree = RBM_NONE; - } currentLocation = newLocation; switch (currentRefPosition->refType) From 9a234a8baa15efc4fcb3933b38d1abaf0fd95912 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 26 Feb 2021 10:00:58 -0800 Subject: [PATCH 3/3] jitformat --- src/coreclr/jit/lsra.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index aa3ebb99105471..db78c5c4e15392 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -10761,7 +10761,7 @@ void LinearScan::verifyFinalAllocation() } LsraLocation newLocation = currentRefPosition->nodeLocation; - currentLocation = newLocation; + currentLocation = newLocation; switch (currentRefPosition->refType) { @@ -11006,11 +11006,11 @@ void LinearScan::verifyFinalAllocation() // Clear the assigned interval of current register. if (interval->physReg != REG_NA && interval->physReg != regNum) { - interval->assignedReg->assignedInterval = nullptr; + interval->assignedReg->assignedInterval = nullptr; } } - interval->physReg = regNum; - interval->assignedReg = regRecord; + interval->physReg = regNum; + interval->assignedReg = regRecord; regRecord->assignedInterval = interval; } }