Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Classify pattern-matching temp as long-lived #18756

Merged
merged 6 commits into from
Apr 19, 2017
+282 −95
Merged

Classify pattern-matching temp as long-lived #18756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a142267
Classify pattern-matching temp as long-lived
gafter Apr 17, 2017
f83d864
Add test demonstrating correct code for spilling pattern termps.
gafter Apr 17, 2017
1e86626
Merge branch 'master' of https://github.com/dotnet/roslyn into master…
gafter Apr 17, 2017
ab4f844
Update test to reflect change in status of pattern temps.
gafter Apr 17, 2017
38a4af1
Test generated code for pattern switch (before temp change)
gafter Apr 18, 2017
437f0e8
Add test for precise IL for pattern switch.
gafter Apr 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/DecisionTree.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public static DecisionTree Create(BoundExpression expression, TypeSymbol type, S
{
// Unless it is a constant, the decision tree acts on a copy of the input expression.
// We create a temp to represent that copy. Lowering will assign into this temp.
temp = new SynthesizedLocal(enclosingSymbol as MethodSymbol, type, SynthesizedLocalKind.PatternMatchingTemp, expression.Syntax, false, RefKind.None);
temp = new SynthesizedLocal(enclosingSymbol as MethodSymbol, type, SynthesizedLocalKind.PatternMatching, expression.Syntax, false, RefKind.None);
expression = new BoundLocal(expression.Syntax, temp, null, type);
}

Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/BoundTree/DecisionTreeBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private DecisionTree AddByValue(DecisionTree.ByType byType, BoundConstantPattern
if (forType == null)
{
var type = value.Value.Type;
var localSymbol = new SynthesizedLocal(_enclosingSymbol as MethodSymbol, type, SynthesizedLocalKind.PatternMatchingTemp, Syntax, false, RefKind.None);
var localSymbol = new SynthesizedLocal(_enclosingSymbol as MethodSymbol, type, SynthesizedLocalKind.PatternMatching, Syntax, false, RefKind.None);
var narrowedExpression = new BoundLocal(Syntax, localSymbol, null, type);
forType = new DecisionTree.ByValue(narrowedExpression, value.Value.Type.TupleUnderlyingTypeOrSelf(), localSymbol);
byType.TypeAndDecision.Add(new KeyValuePair<TypeSymbol, DecisionTree>(value.Value.Type, forType));
Expand Down Expand Up @@ -329,7 +329,7 @@ private DecisionTree AddByType(DecisionTree.ByType byType, TypeSymbol type, Deci

if (result == null)
{
var localSymbol = new SynthesizedLocal(_enclosingSymbol as MethodSymbol, type, SynthesizedLocalKind.PatternMatchingTemp, Syntax, false, RefKind.None);
var localSymbol = new SynthesizedLocal(_enclosingSymbol as MethodSymbol, type, SynthesizedLocalKind.PatternMatching, Syntax, false, RefKind.None);
var expression = new BoundLocal(Syntax, localSymbol, null, type);
result = makeDecision(expression, type);
Debug.Assert(result.Temp == null);
Expand Down
83 changes: 83 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4952,5 +4952,88 @@ namespace System.Runtime.CompilerServices { class AsyncMethodBuilderAttribute :
Diagnostic(ErrorCode.ERR_BadAsyncMethodBuilderTaskProperty, "{ await Task.Delay(5); throw null; }").WithArguments("MyTaskBuilder", "MyTask", "int").WithLocation(7, 29)
);
}

[Fact, WorkItem(18257, "https://github.com/dotnet/roslyn/issues/18257")]
public void PatternTempsAreLongLived()
{
var source = @"using System;

public class Foo {}

public class C {
public static void Main(string[] args)
{
var c = new C();
c.M(new Foo());
c.M(new object());
}
public async void M(object o) {
switch (o)
{
case Foo _:
Console.Write(0);
break;
default:
Console.Write(1);
break;
}
}
}";
var expectedOutput = @"01";
var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe);
base.CompileAndVerify(compilation, expectedOutput: expectedOutput);
}

[Fact, WorkItem(18257, "https://github.com/dotnet/roslyn/issues/18257")]
public void PatternTempsSpill()
{
// This test exercises the spilling machinery of async for pattern-matching temps
var source = @"using System;
using System.Threading.Tasks;

public class C {
public class Foo
{
public int Value;
}
public static void Main(string[] args)
{
var c = new C();
c.M(new Foo() { Value = 1 });
c.M(new Foo() { Value = 2 });
c.M(new Foo() { Value = 3 });
c.M(new object());
}
public void M(object o)
{
MAsync(o).Wait();
}
public async Task MAsync(object o) {
switch (o)
{
case Foo foo when await Copy(foo.Value) == 1:
Console.Write($""{foo.Value}=1 "");
break;
case Foo foo when await Copy(foo.Value) == 2:
Console.Write($""{foo.Value}=2 "");
break;
case Foo foo:
Console.Write($""{foo.Value} "");
break;
default:
Console.Write(""X "");
break;
}
}
public async Task<int> Copy(int i)
{
await Task.Delay(1);
return i;
}
}";
var expectedOutput = @"1=1 2=2 3 X";
var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe);
base.CompileAndVerify(compilation, expectedOutput: expectedOutput);
}
}
}
147 changes: 133 additions & 14 deletions src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2705,9 +2705,9 @@ class Student : Person { public double GPA; }
<customDebugInfo>
<forward declaringType=""Program"" methodName=""Main"" parameterNames=""args"" />
<encLocalSlotMap>
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""11"" />
<slot kind=""35"" offset=""46"" />
<slot kind=""35"" offset=""237"" />
<slot kind=""temp"" />
<slot kind=""0"" offset=""59"" />
<slot kind=""0"" offset=""163"" />
Expand Down Expand Up @@ -2798,9 +2798,9 @@ class Student : Person { public double GPA; }
<encLocalSlotMap>
<slot kind=""30"" offset=""0"" />
<slot kind=""30"" offset=""383"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""11"" />
<slot kind=""35"" offset=""46"" />
<slot kind=""35"" offset=""249"" />
<slot kind=""temp"" />
<slot kind=""1"" offset=""11"" />
<slot kind=""21"" offset=""0"" />
Expand Down Expand Up @@ -5827,6 +5827,122 @@ static void M(object o)
}
}";
var c = CreateCompilationWithMscorlibAndSystemCore(source, options: TestOptions.DebugDll);
CompileAndVerify(c).VerifyIL("Program.M",
@"{
// Code size 210 (0xd2)
.maxstack 2
.locals init (object V_0,
int V_1,
object V_2,
object V_3,
int? V_4,
int V_5,
object V_6,
int V_7,
object V_8,
object V_9,
object V_10)
IL_0000: nop
IL_0001: ldarg.0
IL_0002: stloc.2
IL_0003: ldloc.2
IL_0004: stloc.3
IL_0005: ldloc.3
IL_0006: stloc.0
IL_0007: ldloc.0
IL_0008: brtrue.s IL_000c
IL_000a: br.s IL_005e
IL_000c: ldloc.0
IL_000d: isinst ""int?""
IL_0012: unbox.any ""int?""
IL_0017: stloc.s V_4
IL_0019: ldloca.s V_4
IL_001b: call ""int int?.GetValueOrDefault()""
IL_0020: stloc.1
IL_0021: ldloca.s V_4
IL_0023: call ""bool int?.HasValue.get""
IL_0028: brfalse.s IL_005e
IL_002a: ldloc.1
IL_002b: stloc.s V_5
IL_002d: ldloc.s V_5
IL_002f: ldc.i4.1
IL_0030: sub
IL_0031: switch (
IL_004c,
IL_0054,
IL_005a,
IL_0052,
IL_0058)
IL_004a: br.s IL_005e
IL_004c: br.s IL_0060
IL_004e: br.s IL_006c
IL_0050: br.s IL_007a
IL_0052: br.s IL_006a
IL_0054: br.s IL_0065
IL_0056: br.s IL_005e
IL_0058: br.s IL_0076
IL_005a: br.s IL_0071
IL_005c: br.s IL_005e
IL_005e: br.s IL_0078
IL_0060: ldarg.0
IL_0061: brfalse.s IL_006a
IL_0063: br.s IL_004e
IL_0065: ldarg.0
IL_0066: brfalse.s IL_006a
IL_0068: br.s IL_0056
IL_006a: br.s IL_007c
IL_006c: ldarg.0
IL_006d: brtrue.s IL_0076
IL_006f: br.s IL_0050
IL_0071: ldarg.0
IL_0072: brtrue.s IL_0076
IL_0074: br.s IL_005c
IL_0076: br.s IL_007c
IL_0078: br.s IL_007c
IL_007a: br.s IL_007c
IL_007c: ldarg.0
IL_007d: stloc.2
IL_007e: ldloc.2
IL_007f: stloc.s V_8
IL_0081: ldloc.s V_8
IL_0083: stloc.s V_6
IL_0085: ldloc.s V_6
IL_0087: brtrue.s IL_008b
IL_0089: br.s IL_00b8
IL_008b: ldloc.s V_6
IL_008d: isinst ""int?""
IL_0092: unbox.any ""int?""
IL_0097: stloc.s V_4
IL_0099: ldloca.s V_4
IL_009b: call ""int int?.GetValueOrDefault()""
IL_00a0: stloc.s V_7
IL_00a2: ldloca.s V_4
IL_00a4: call ""bool int?.HasValue.get""
IL_00a9: brfalse.s IL_00b8
IL_00ab: ldloc.s V_7
IL_00ad: stloc.s V_5
IL_00af: ldloc.s V_5
IL_00b1: ldc.i4.1
IL_00b2: beq.s IL_00b6
IL_00b4: br.s IL_00b8
IL_00b6: br.s IL_00ba
IL_00b8: br.s IL_00bc
IL_00ba: br.s IL_00be
IL_00bc: br.s IL_00be
IL_00be: ldarg.0
IL_00bf: stloc.2
IL_00c0: ldloc.2
IL_00c1: stloc.s V_10
IL_00c3: ldloc.s V_10
IL_00c5: stloc.s V_9
IL_00c7: ldloc.s V_9
IL_00c9: brtrue.s IL_00cd
IL_00cb: br.s IL_00cd
IL_00cd: br.s IL_00cf
IL_00cf: br.s IL_00d1
IL_00d1: ret
}
");
c.VerifyPdb(
@"<symbols>
<methods>
Expand All @@ -5836,13 +5952,16 @@ static void M(object o)
<namespace usingCount=""0"" />
</using>
<encLocalSlotMap>
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""11"" />
<slot kind=""35"" offset=""46"" />
<slot kind=""temp"" />
<slot kind=""1"" offset=""11"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""378"" />
<slot kind=""35"" offset=""413"" />
<slot kind=""1"" offset=""378"" />
<slot kind=""35"" offset=""511"" />
<slot kind=""1"" offset=""511"" />
</encLocalSlotMap>
</customDebugInfo>
Expand All @@ -5860,12 +5979,12 @@ static void M(object o)
<entry offset=""0x7a"" startLine=""18"" startColumn=""17"" endLine=""18"" endColumn=""23"" />
<entry offset=""0x7c"" startLine=""20"" startColumn=""9"" endLine=""20"" endColumn=""19"" />
<entry offset=""0x81"" hidden=""true"" />
<entry offset=""0xb5"" startLine=""23"" startColumn=""17"" endLine=""23"" endColumn=""23"" />
<entry offset=""0xb7"" startLine=""25"" startColumn=""17"" endLine=""25"" endColumn=""23"" />
<entry offset=""0xb9"" startLine=""27"" startColumn=""9"" endLine=""27"" endColumn=""19"" />
<entry offset=""0xbe"" hidden=""true"" />
<entry offset=""0xc8"" startLine=""30"" startColumn=""17"" endLine=""30"" endColumn=""23"" />
<entry offset=""0xca"" startLine=""32"" startColumn=""5"" endLine=""32"" endColumn=""6"" />
<entry offset=""0xba"" startLine=""23"" startColumn=""17"" endLine=""23"" endColumn=""23"" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in sequence points suggest there are changes in IL. Seems like no other test covers the difference in IL. It would be good to have one.

<entry offset=""0xbc"" startLine=""25"" startColumn=""17"" endLine=""25"" endColumn=""23"" />
<entry offset=""0xbe"" startLine=""27"" startColumn=""9"" endLine=""27"" endColumn=""19"" />
<entry offset=""0xc3"" hidden=""true"" />
<entry offset=""0xcf"" startLine=""30"" startColumn=""17"" endLine=""30"" endColumn=""23"" />
<entry offset=""0xd1"" startLine=""32"" startColumn=""5"" endLine=""32"" endColumn=""6"" />
</sequencePoints>
</method>
</methods>
Expand Down
15 changes: 9 additions & 6 deletions src/Compilers/Core/Portable/SynthesizedLocalKind.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ internal enum SynthesizedLocalKind
/// </summary>
FrameCache = -5,

/// <summary>
/// Temp created for pattern matching by type.
/// </summary>
PatternMatchingTemp = -4,

/// <summary>
/// Temp variable created by the optimizer.
/// </summary>
Expand All @@ -53,7 +48,9 @@ internal enum SynthesizedLocalKind
EmitterTemp = -1,

/// <summary>
/// The variable is not synthesized (C#, VB).
/// The variable is not synthesized (C#, VB). Note that SynthesizedLocalKind values
/// greater than or equal to this are considered long-lived;
/// see <see cref="SynthesizedLocalKindExtensions.IsLongLived"/>.
/// </summary>
UserDefined = 0,

Expand Down Expand Up @@ -206,6 +203,12 @@ internal enum SynthesizedLocalKind
/// </summary>
InstrumentationPayload = 34,

/// <summary>
/// Temp created for pattern matching by type. This holds the value of an input value provisionally
/// converted to the type against which it is being matched.
/// </summary>
PatternMatching = 35,

/// <summary>
/// All values have to be less than or equal to <see cref="MaxValidValueForLocalVariableSerializedToDebugInformation"/>
/// (<see cref="EditAndContinueMethodDebugInformation"/>)
Expand Down
Loading