提交 252d29a1 编写于 作者: V vsadov

Fixes a subtle bug in branch optimizer.

"branch over branch" optimization eliminates chains of trivial basic blocks from the graph, which is correct as long as trivial blocks are not reachable individually from outside via branches.
The "same as next" optimization can make a block trivial and if it was targeted by a branch, break the assumption, leading to a rare situation of some branches having stale offsets.

The fix makes sure if a block becomes trivial, its labels are moved to the next nontrivial block.

Fixes #4838
Fixes #4839
上级 7cdd08a1
......@@ -3463,5 +3463,175 @@ public void AwaitInStaticInitializer()
// static int x = await System.Threading.Tasks.Task.FromResult(1);
Diagnostic(ErrorCode.ERR_BadAwaitWithoutAsync, "await System.Threading.Tasks.Task.FromResult(1)").WithLocation(1, 16));
}
[Fact, WorkItem(4839, "https://github.com/dotnet/roslyn/issues/4839")]
public void SwitchOnAwaitedValueAsync()
{
var source = @"
using System.Threading.Tasks;
using System;
class Program
{
static void Main()
{
M(0).Wait();
}
static async Task M(int input)
{
var value = 1;
switch (value)
{
case 0:
return;
case 1:
return;
}
}
}
";
var comp = CreateCompilation(source, options: TestOptions.DebugExe);
CompileAndVerify(comp);
CompileAndVerify(comp.WithOptions(TestOptions.ReleaseExe));
}
[Fact, WorkItem(4839, "https://github.com/dotnet/roslyn/issues/4839")]
public void SwitchOnAwaitedValue()
{
var source = @"
using System.Threading.Tasks;
using System;
class Program
{
static void Main()
{
M(0);
}
static void M(int input)
{
try
{
var value = 1;
switch (value)
{
case 1:
return;
case 2:
return;
}
}
catch (Exception)
{
}
}
}
";
var comp = CreateCompilation(source, options: TestOptions.ReleaseExe);
CompileAndVerify(comp).
VerifyIL("Program.M(int)",
@"
{
// Code size 16 (0x10)
.maxstack 2
.locals init (int V_0) //value
.try
{
IL_0000: ldc.i4.1
IL_0001: stloc.0
IL_0002: ldloc.0
IL_0003: ldc.i4.1
IL_0004: beq.s IL_000a
IL_0006: ldloc.0
IL_0007: ldc.i4.2
IL_0008: pop
IL_0009: pop
IL_000a: leave.s IL_000f
}
catch System.Exception
{
IL_000c: pop
IL_000d: leave.s IL_000f
}
IL_000f: ret
}
");
}
[Fact, WorkItem(4839, "https://github.com/dotnet/roslyn/issues/4839")]
public void SwitchOnAwaitedValueString()
{
var source = @"
using System.Threading.Tasks;
using System;
class Program
{
static void Main()
{
M(0).Wait();
}
static async Task M(int input)
{
var value = ""q"";
switch (value)
{
case ""a"":
return;
case ""b"":
return;
}
}
}
";
var comp = CreateCompilation(source, options: TestOptions.DebugExe);
CompileAndVerify(comp);
CompileAndVerify(comp.WithOptions(TestOptions.ReleaseExe));
}
[Fact, WorkItem(4838, "https://github.com/dotnet/roslyn/issues/4838")]
public void SwitchOnAwaitedValueInLoop()
{
var source = @"
using System.Threading.Tasks;
using System;
class Program
{
static void Main()
{
M(0).Wait();
}
static async Task M(int input)
{
for (;;)
{
var value = await Task.FromResult(input);
switch (value)
{
case 0:
return;
case 3:
return;
case 4:
continue;
case 100:
return;
default:
throw new ArgumentOutOfRangeException(""Unknown value: "" + value);
}
}
}
}
";
var comp = CreateCompilation(source, options: TestOptions.DebugExe);
CompileAndVerify(comp);
CompileAndVerify(comp.WithOptions(TestOptions.ReleaseExe));
}
}
}
......@@ -408,6 +408,26 @@ private bool TryOptimizeSameAsNext(BasicBlock next, ref int delta)
var diff = this.BranchCode.Size() + this.BranchCode.BranchOperandSize();
delta -= diff;
this.SetBranch(null, ILOpCode.Nop);
// If current block has no regular instructions the resulting block is a trivial noop
// TryOptimizeBranchOverUncondBranch relies on an invariant that
// trivial blocks are not targeted by branches,
// make sure we are not breaking this condition.
if (this.HasNoRegularInstructions)
{
var labelInfos = builder._labelInfos;
var labels = labelInfos.Keys;
foreach (var label in labels)
{
var info = labelInfos[label];
if (info.bb == this)
{
// move the label from "this" to "next"
labelInfos[label] = info.WithNewTarget(next);
}
}
}
return true;
}
}
......@@ -427,19 +447,18 @@ private bool TryOptimizeBranchOverUncondBranch(BasicBlock next, ref int delta)
if (revBrOp != ILOpCode.Nop)
{
// we are effectively removing "next" from the block chain.
// that is ok, since branch-to-branch should already eliminate any possible branches to "next"
// and it was only reachable from current via NextBlock which we are re-directing.
// Also, if there are any blocks between "next" and BranchBlock, they are all empty
// so we do not even care if they are reachable or not.
Debug.Assert(!builder._labelInfos.Values.Any(li => li.bb == next), "nothing should branch to a branch at this point");
var intermediateNext = this.NextBlock;
while (intermediateNext != next)
// we are effectively removing blocks between this and the BranchBlock (including next) from the block chain.
// that is ok, since branch-to-branch should already eliminate any possible branches to these blocks
// and they were only reachable from current via NextBlock which we are re-directing.
var toRemove = this.NextBlock;
var branchBlock = this.BranchBlock;
while (toRemove != branchBlock)
{
Debug.Assert(intermediateNext.TotalSize == 0);
intermediateNext.Reachability = ILBuilder.Reachability.NotReachable;
intermediateNext = intermediateNext.NextBlock;
Debug.Assert(toRemove == next || toRemove.TotalSize == 0);
Debug.Assert(!builder._labelInfos.Values.Any(li => li.bb == toRemove),
"nothing should branch to a trivial block at this point");
toRemove.Reachability = ILBuilder.Reachability.NotReachable;
toRemove = toRemove.NextBlock;
}
next.Reachability = Reachability.NotReachable;
......
......@@ -814,7 +814,7 @@ private bool ComputeOffsetsAndAdjustBranches()
if (_optimizations == OptimizationLevel.Release)
{
branchesOptimized |= current.OptimizeBranches(ref delta);
branchesOptimized |= current.OptimizeBranches(ref delta);
}
current.ShortenBranches(ref delta);
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册