diff --git a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs
index e8d593ec393d4b58b8e8d36fe36c17a11b47a63d..bae012123fc3226fe7c20a983c81a53d0098f97b 100644
--- a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs
+++ b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs
@@ -524,13 +524,7 @@ DagState uniqifyState(DagState state)
}
else
{
- BoundDagDecision selectDecision(DagState s)
- {
- // Our simple heuristic is to perform the first test of the first possible matched case
- return s.Cases[0].Decisions[0];
- }
-
- switch (state.SelectedDecision = selectDecision(state))
+ switch (state.SelectedDecision = state.ComputeSelectedDecision())
{
case BoundDagEvaluation e:
state.TrueBranch = uniqifyState(RemoveEvaluation(state.Cases, e));
@@ -643,6 +637,7 @@ BoundDecisionDag finalState(SyntaxNode syntax, LabelSymbol label, ImmutableArray
finalStates.Free();
Debug.Assert(initialState.Dag != null);
+ // Note: It is useful for debugging the dag state table construction to view `initialState.Dump()` here.
return initialState.Dag;
}
@@ -775,6 +770,10 @@ BoundDecisionDag finalState(SyntaxNode syntax, LabelSymbol label, ImmutableArray
trueDecisionPermitsTrueOther = false; // if v!=null is true, then v==null cannot succeed
falseDecisionImpliesTrueOther = true; // if v!=null is false, then v==null has been proven true
break;
+ case BoundNonNullDecision n2:
+ trueDecisionImpliesTrueOther = true;
+ falseDecisionPermitsTrueOther = false;
+ break;
default:
// Once v!=null fails, it must fail a type test
falseDecisionPermitsTrueOther = false;
@@ -885,6 +884,10 @@ private static PartialCaseDecision RemoveEvaluation(PartialCaseDecision c, Bound
Bindings: c.Bindings, WhenClause: c.WhenClause, CaseLabel: c.CaseLabel);
}
+ ///
+ /// The state at a given node of the decision acyclic graph. This is used during computation of the state machine,
+ /// and contains a representation of the meaning of the state.
+ ///
private class DagState
{
public readonly ImmutableArray Cases;
@@ -904,7 +907,134 @@ public DagState(ImmutableArray cases)
// After the entire graph of DagState objects is complete, we translate each into its Dag.
public BoundDecisionDag Dag;
+
+ // Compute a decision to use at the root of the generated decision tree.
+ internal BoundDagDecision ComputeSelectedDecision()
+ {
+ // Our simple heuristic is to perform the first test of the first possible matched case
+ var choice = Cases[0].Decisions[0];
+
+ // But if that test is a null check, it would be redundant with a following
+ // type test. We apply this refinement only when there is exactly one case, because
+ // when there are multiple cases the null check is likely to be shared.
+ if (choice.Kind == BoundKind.NonNullDecision &&
+ Cases.Length == 1 &&
+ Cases[0].Decisions.Length > 1)
+ {
+ var choice2 = Cases[0].Decisions[1];
+ if (choice2.Kind == BoundKind.TypeDecision)
+ {
+ return choice2;
+ }
+ }
+
+ return choice;
+ }
+
+#if DEBUG
+ ///
+ /// Starting with `this` state, produce a human-readable description of the state tables.
+ /// This is very useful for debugging and optimizing the dag state construction.
+ ///
+ internal string Dump()
+ {
+ var printed = PooledHashSet.GetInstance();
+
+ int nextStateNumber = 0;
+ var workQueue = ArrayBuilder.GetInstance();
+ var stateIdentifierMap = PooledDictionary.GetInstance();
+ int stateIdentifier(DagState state)
+ {
+ if (stateIdentifierMap.TryGetValue(state, out int value))
+ {
+ return value;
+ }
+ else
+ {
+ value = stateIdentifierMap[state] = ++nextStateNumber;
+ workQueue.Push(state);
+ }
+
+ return value;
+ }
+
+ int nextTempNumber = 0;
+ var tempIdentifierMap = PooledDictionary.GetInstance();
+ int tempIdentifier(BoundDagEvaluation e)
+ {
+ return (e == null) ? 0 : tempIdentifierMap.TryGetValue(e, out int value) ? value : tempIdentifierMap[e] = ++nextTempNumber;
+ }
+
+ string tempName(BoundDagTemp t)
+ {
+ return $"t{tempIdentifier(t.Source)}{(t.Index != 0 ? $".{t.Index.ToString()}" : "")}";
+ }
+
+ var resultBuilder = PooledStringBuilder.GetInstance();
+ var result = resultBuilder.Builder;
+ stateIdentifier(this); // push the start node onto the work queue
+
+ while (workQueue.Count != 0)
+ {
+ var state = workQueue.Pop();
+ if (!printed.Add(state))
+ {
+ continue;
+ }
+
+ result.AppendLine($"State " + stateIdentifier(state));
+ foreach (PartialCaseDecision cd in state.Cases)
+ {
+ result.Append($" [{cd.Syntax}]");
+ foreach (BoundDagDecision d in cd.Decisions)
+ {
+ result.Append($" {dump(d)}");
+ }
+
+ result.AppendLine();
+ }
+
+ if (state.SelectedDecision != null)
+ {
+ result.AppendLine($" Decision: {dump(state.SelectedDecision)}");
+ }
+
+ if (state.TrueBranch != null)
+ {
+ result.AppendLine($" TrueBranch: {stateIdentifier(state.TrueBranch)}");
+ }
+
+ if (state.FalseBranch != null)
+ {
+ result.AppendLine($" FalseBranch: {stateIdentifier(state.FalseBranch)}");
+ }
+ }
+
+ workQueue.Free();
+ printed.Free();
+ stateIdentifierMap.Free();
+ tempIdentifierMap.Free();
+ return resultBuilder.ToStringAndFree();
+
+ string dump(BoundDagDecision d)
+ {
+ switch (d)
+ {
+ case BoundDagTypeEvaluation a:
+ return $"t{tempIdentifier(a)}={a.Kind}({a.Type.ToString()})";
+ case BoundDagEvaluation e:
+ return $"t{tempIdentifier(e)}={e.Kind}";
+ case BoundTypeDecision b:
+ return $"?{d.Kind}({b.Type.ToString()}, {tempName(d.Input)})";
+ case BoundNonNullValueDecision v:
+ return $"?{d.Kind}({v.Value.ToString()}, {tempName(d.Input)})";
+ default:
+ return $"?{d.Kind}({tempName(d.Input)})";
+ }
+ }
+ }
}
+#endif
///
/// An equivalence relation between dag states used to dedup the states during dag construction.
diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs
index ef89f04698e35f43892c4ff438851df390465696..3d87b1742f7b023b426760fdee66fa59a3f96c8c 100644
--- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs
+++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs
@@ -149,6 +149,7 @@ private void LowerDecisionDag(ImmutableArray sortedNodes)
///
/// Translate the decision tree for node, given that it will be followed by the translation for nextNode.
+ /// This knowledge allows us to fall through to nextNode instead of producing a goto statement to get there.
///
private void LowerDecisionDagNode(BoundDecisionDag node, BoundDecisionDag nextNode)
{
diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs
index 8fec2fa33c62f22fd5141bc2b3328a43155270f9..99a663d78c42cd255e301072c076eb5ba4c201c8 100644
--- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs
+++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs
@@ -203,7 +203,6 @@ protected BoundExpression LowerDecision(BoundDagDecision decision)
}
else
{
- // PROTOTYPE(patterns2): combine null test and type test when possible for improved code
return _localRewriter.MakeNullCheck(d.Syntax, input, input.Type.IsNullableType() ? BinaryOperatorKind.NullableNullNotEqual : BinaryOperatorKind.NotEqual);
}
@@ -211,6 +210,7 @@ protected BoundExpression LowerDecision(BoundDagDecision decision)
case BoundTypeDecision d:
{
+ // Note that this tests for non-null as a side-effect. We depend on that to sometimes avoid the null check.
return _factory.Is(input, d.Type);
}
diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs
index 63256a687b2a8eb0eb713636033dfa95f61451b4..00fcdfa1190199d672d245ff1aa6852f57011a15 100644
--- a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs
+++ b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs
@@ -142,7 +142,6 @@ static void M1(int? x)
);
}
- // PROTOTYPE(patterns2): code quality has regressed for this test because the lowering strategy reifies the boolean result of the if expression.
[Fact, WorkItem(17266, "https://github.com/dotnet/roslyn/issues/17266")]
public void DoubleEvaluation01()
{
@@ -200,6 +199,28 @@ .maxstack 1
IL_002a: nop
IL_002b: ret
}");
+
+ compilation = CreateCompilation(source, options: TestOptions.ReleaseExe);
+ compilation.VerifyDiagnostics();
+ compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
+ compVerifier.VerifyIL("C.Main",
+@"{
+ // Code size 30 (0x1e)
+ .maxstack 1
+ .locals init (int V_0, //index
+ int? V_1)
+ IL_0000: call ""int? C.TryGet()""
+ IL_0005: stloc.1
+ IL_0006: ldloca.s V_1
+ IL_0008: call ""bool int?.HasValue.get""
+ IL_000d: brfalse.s IL_001d
+ IL_000f: ldloca.s V_1
+ IL_0011: call ""int int?.GetValueOrDefault()""
+ IL_0016: stloc.0
+ IL_0017: ldloc.0
+ IL_0018: call ""void System.Console.WriteLine(int)""
+ IL_001d: ret
+}");
}
[Fact, WorkItem(19122, "https://github.com/dotnet/roslyn/issues/19122")]
@@ -248,7 +269,7 @@ public Guid GetId(T t)
var compVerifier = CompileAndVerify(compilation);
compVerifier.VerifyIL("X.Y",
@"{
- // Code size 71 (0x47)
+ // Code size 63 (0x3f)
.maxstack 3
.locals init (U V_0, //u
bool V_1,
@@ -264,26 +285,23 @@ .maxstack 3
IL_0018: stloc.2
IL_0019: ldloc.2
IL_001a: box ""T""
- IL_001f: brfalse.s IL_003f
- IL_0021: ldloc.2
- IL_0022: box ""T""
- IL_0027: isinst ""U""
- IL_002c: brfalse.s IL_003f
- IL_002e: ldloc.2
- IL_002f: box ""T""
- IL_0034: unbox.any ""U""
- IL_0039: stloc.3
- IL_003a: ldloc.3
- IL_003b: stloc.0
- IL_003c: ldc.i4.1
- IL_003d: br.s IL_0040
- IL_003f: ldc.i4.0
- IL_0040: stloc.1
- IL_0041: ldloc.1
- IL_0042: brfalse.s IL_0046
- IL_0044: nop
- IL_0045: nop
- IL_0046: ret
+ IL_001f: isinst ""U""
+ IL_0024: brfalse.s IL_0037
+ IL_0026: ldloc.2
+ IL_0027: box ""T""
+ IL_002c: unbox.any ""U""
+ IL_0031: stloc.3
+ IL_0032: ldloc.3
+ IL_0033: stloc.0
+ IL_0034: ldc.i4.1
+ IL_0035: br.s IL_0038
+ IL_0037: ldc.i4.0
+ IL_0038: stloc.1
+ IL_0039: ldloc.1
+ IL_003a: brfalse.s IL_003e
+ IL_003c: nop
+ IL_003d: nop
+ IL_003e: ret
}");
}
@@ -327,7 +345,7 @@ static void Main(string[] args)
var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
compVerifier.VerifyIL("Program.Main",
@"{
- // Code size 113 (0x71)
+ // Code size 106 (0x6a)
.maxstack 2
.locals init (Base