Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
- Change gtIsLikelyRegVar to return false for defs of EHvars, since…
Browse files Browse the repository at this point in the history
… they always go to memory.

- Never split a block if the only resolution moves are for EH vars.
- Add a test case to enable diff tracking for #21973
  • Loading branch information
CarolEidt committed Nov 15, 2019
1 parent f00cb24 commit c2ae4dd
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 34 deletions.
7 changes: 7 additions & 0 deletions src/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2755,6 +2755,13 @@ bool Compiler::gtIsLikelyRegVar(GenTree* tree)
return false;
}

// If this is an EH-live var, return false if it is a def,
// as it will have to go to memory.
if (varDsc->lvLiveInOutOfHndlr && ((tree->gtFlags & GTF_VAR_DEF) != 0))
{
return false;
}

// Be pessimistic if ref counts are not yet set up.
//
// Perhaps we should be optimistic though.
Expand Down
25 changes: 24 additions & 1 deletion src/jit/lsra.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8232,7 +8232,30 @@ void LinearScan::handleOutgoingCriticalEdges(BasicBlock* block)
}
if (!VarSetOps::IsEmpty(compiler, edgeResolutionSet))
{
resolveEdge(block, succBlock, ResolveCritical, edgeResolutionSet);
// For EH vars, we can always safely load them from the stack into the target for this block,
// so if we have only EH vars, we'll do that instead of splitting the edge.
if ((compiler->compHndBBtabCount > 0) && VarSetOps::IsSubset(compiler, edgeResolutionSet, exceptVars))
{
GenTree* insertionPoint = LIR::AsRange(succBlock).FirstNonPhiNode();
VarSetOps::Iter edgeSetIter(compiler, edgeResolutionSet);
unsigned edgeVarIndex = 0;
while (edgeSetIter.NextElem(&edgeVarIndex))
{
regNumber toReg = getVarReg(succInVarToRegMap, edgeVarIndex);
setVarReg(succInVarToRegMap, edgeVarIndex, REG_STK);
if (toReg != REG_STK)
{
Interval* interval = getIntervalForLocalVar(edgeVarIndex);
assert(interval->isWriteThru);
addResolution(succBlock, insertionPoint, interval, toReg, REG_STK);
JITDUMP(" (EHvar)\n");
}
}
}
else
{
resolveEdge(block, succBlock, ResolveCritical, edgeResolutionSet);
}
}
}
}
Expand Down
63 changes: 30 additions & 33 deletions src/jit/lsrabuild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2248,49 +2248,46 @@ void LinearScan::buildIntervals()

if (enregisterLocalVars)
{
// We don't need exposed uses for an EH edge, because no lclVars will be kept in
// registers across such edges.
if (!blockInfo[block->bbNum].hasEHBoundaryOut)
// Insert exposed uses for a lclVar that is live-out of 'block' but not live-in to the
// next block, or any unvisited successors.
// This will address lclVars that are live on a backedge, as well as those that are kept
// live at a GT_JMP.
//
// Blocks ending with "jmp method" are marked as BBJ_HAS_JMP,
// and jmp call is represented using GT_JMP node which is a leaf node.
// Liveness phase keeps all the arguments of the method live till the end of
// block by adding them to liveout set of the block containing GT_JMP.
//
// The target of a GT_JMP implicitly uses all the current method arguments, however
// there are no actual references to them. This can cause LSRA to assert, because
// the variables are live but it sees no references. In order to correctly model the
// liveness of these arguments, we add dummy exposed uses, in the same manner as for
// backward branches. This will happen automatically via expUseSet.
//
// Note that a block ending with GT_JMP has no successors and hence the variables
// for which dummy use ref positions are added are arguments of the method.

VARSET_TP expUseSet(VarSetOps::MakeCopy(compiler, block->bbLiveOut));
VarSetOps::IntersectionD(compiler, expUseSet, registerCandidateVars);
BasicBlock* nextBlock = getNextBlock();
if (nextBlock != nullptr)
{
// Insert exposed uses for a lclVar that is live-out of 'block' but not live-in to the
// next block, or any unvisited successors.
// This will address lclVars that are live on a backedge, as well as those that are kept
// live at a GT_JMP.
//
// Blocks ending with "jmp method" are marked as BBJ_HAS_JMP,
// and jmp call is represented using GT_JMP node which is a leaf node.
// Liveness phase keeps all the arguments of the method live till the end of
// block by adding them to liveout set of the block containing GT_JMP.
//
// The target of a GT_JMP implicitly uses all the current method arguments, however
// there are no actual references to them. This can cause LSRA to assert, because
// the variables are live but it sees no references. In order to correctly model the
// liveness of these arguments, we add dummy exposed uses, in the same manner as for
// backward branches. This will happen automatically via expUseSet.
//
// Note that a block ending with GT_JMP has no successors and hence the variables
// for which dummy use ref positions are added are arguments of the method.

VARSET_TP expUseSet(VarSetOps::MakeCopy(compiler, block->bbLiveOut));
VarSetOps::IntersectionD(compiler, expUseSet, registerCandidateVars);
BasicBlock* nextBlock = getNextBlock();
if (nextBlock != nullptr)
VarSetOps::DiffD(compiler, expUseSet, nextBlock->bbLiveIn);
}
for (BasicBlock* succ : block->GetAllSuccs(compiler))
{
if (VarSetOps::IsEmpty(compiler, expUseSet))
{
VarSetOps::DiffD(compiler, expUseSet, nextBlock->bbLiveIn);
break;
}
for (BasicBlock* succ : block->GetAllSuccs(compiler))
{
if (VarSetOps::IsEmpty(compiler, expUseSet))
{
break;
}

if (isBlockVisited(succ))
{
continue;
}
VarSetOps::DiffD(compiler, expUseSet, succ->bbLiveIn);
}

if (!VarSetOps::IsEmpty(compiler, expUseSet))
{
JITDUMP("Exposed uses:");
Expand Down
24 changes: 24 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_21973/GitHub_21973.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Threading.Tasks;
using System.Runtime.CompilerServices;

public class Test
{

[MethodImpl(MethodImplOptions.NoInlining)]
public static async Task CompletedTask()
{
for (int i = 0; i < 100; i++)
await Task.CompletedTask;
}

public static int Main()
{
CompletedTask();
return 100;
}
}
13 changes: 13 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_21973/GitHub_21973.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<PropertyGroup>
<DebugType />
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit c2ae4dd

Please sign in to comment.