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

Jump to bottom

Generate branchless IL for (b ? 1 : 0) #67191

Merged
merged 34 commits into from Apr 17, 2023
Merged

Generate branchless IL for (b ? 1 : 0) #67191

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
26b944a
Add tests
jjonescz Mar 6, 2023
e2c3f4d
Generate branchless IL for `(b ? 1 : 0)`
jjonescz Mar 6, 2023
01f7f64
Update IL snapshots
jjonescz Mar 6, 2023
333d1aa
Move BuildValidator to 64 bit
jaredpar Mar 8, 2023
1fe26ef
Rename `isOne` out parameter
jjonescz Mar 9, 2023
87a5313
Add nonbinary test
jjonescz Mar 13, 2023
bb482c5
Restrict the optimization
jjonescz Mar 13, 2023
513097c
Test IL in Debug
jjonescz Mar 13, 2023
62cc9d9
Encapsulate the emit into method
jjonescz Mar 14, 2023
50d7ae7
Convert result
jjonescz Mar 14, 2023
f5bda52
Disable optimization in Debug
jjonescz Mar 14, 2023
270c816
Improve wording
jjonescz Mar 14, 2023
6344f0c
Check type
jjonescz Mar 14, 2023
97e88fa
Rename emit comparison method
jjonescz Mar 15, 2023
3f19b46
Clarify test options
jjonescz Mar 15, 2023
9bee6a7
Extend tests
jjonescz Mar 15, 2023
3270493
Revert "Move BuildValidator to 64 bit"
jjonescz Mar 15, 2023
853b7bf
Merge branch 'main' into 61483-Emit-cgt
jjonescz Mar 15, 2023
49e600c
Optimize `char`s
jjonescz Mar 16, 2023
435381b
Remove unnecessary `char` check
jjonescz Mar 17, 2023
9c45b18
Extend `char` tests
jjonescz Mar 17, 2023
08e5bf0
Add VB tests
jjonescz Mar 20, 2023
3a4b3c4
Emit branchless IL for `If(b, 1, 0)` in VB
jjonescz Mar 20, 2023
4e09301
Update IL snapshot
jjonescz Mar 20, 2023
ff8b135
Add sequence points to IL snapshots
jjonescz Mar 21, 2023
65e3db4
Test `isinst`
jjonescz Mar 21, 2023
857b0c4
Optimize `isinst`
jjonescz Mar 21, 2023
1f0cc3a
Optimize `is` codegen when used inside condition
jjonescz Mar 21, 2023
e9aca83
Rename parameter which omits boolean conversion of `is` codegen
jjonescz Mar 22, 2023
f2c8d13
Clarify return values
jjonescz Apr 14, 2023
be1432c
Improve variable naming
jjonescz Apr 14, 2023
d202e2b
Test IntPtr in VB
jjonescz Apr 14, 2023
c3e6928
Test `ILEmitStyle.DebugFriendlyRelease`
jjonescz Apr 14, 2023
72b4890
Merge branch 'main' into 61483-Emit-cgt
jjonescz Apr 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
View file
Open in desktop
Expand Up @@ -3483,6 +3483,16 @@ private void EmitConditionalOperator(BoundConditionalOperator expr, bool used)
{
Debug.Assert(expr.ConstantValueOpt == null, "Constant value should have been emitted directly");

// Generate branchless IL for (b ? 1 : 0).
if (used &&
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
hasIntegralValueZeroOrOne(expr.Consequence, out var one1) &&
hasIntegralValueZeroOrOne(expr.Alternative, out var one2) &&
one1 != one2)
{
EmitCondExpr(expr.Condition, sense: one1);
return;
}

object consequenceLabel = new object();
object doneLabel = new object();

Expand Down Expand Up @@ -3547,6 +3557,27 @@ private void EmitConditionalOperator(BoundConditionalOperator expr, bool used)
}

_builder.MarkLabel(doneLabel);

static bool hasIntegralValueZeroOrOne(BoundExpression expr, out bool one)
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
{
if (expr.ConstantValueOpt is { } constantValue)
{
if (constantValue is { IsIntegral: true, UInt64Value: (1 or 0) and var v })
Copy link
Member

@jcouv jcouv Apr 12, 2023 •
edited

Choose a reason for hiding this comment

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

v

nit: I don't love the single-letter variable names, but in this code it's not too bad (each is only used once). But consider naming this one i ;-) #Closed

{
one = v == 1;
return true;
}

if (constantValue is { IsBoolean: true, BooleanValue: var b })
{
one = b;
return true;
}
}

one = false;
return false;
}
}

/// <summary>
Expand Down
160 changes: 147 additions & 13 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenConditionalOperatorTests.cs
View file
Open in desktop
Expand Up @@ -13,6 +13,144 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen
{
public class CodeGenConditionalOperatorTests : CSharpTestBase
{
[Fact, WorkItem(61483, "https://github.com/dotnet/roslyn/issues/61483")]
public void Branchless_Compare()
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
{
var source = """
using static System.Console;
WriteLine(C.Compare(1, 2));
WriteLine(C.Compare(3, 3));
WriteLine(C.Compare(5, 4));

class C
{
public static int Compare(int x, int y)
{
int tmp1 = (x > y) ? 1 : 0;
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
int tmp2 = (x < y) ? 1 : 0;
return tmp1 - tmp2;
}
}
""";
var verifier = CompileAndVerify(source, expectedOutput: """
-1
0
1
""");
verifier.VerifyDiagnostics();
verifier.VerifyMethodBody("C.Compare", """
{
// Code size 12 (0xc)
.maxstack 3
.locals init (int V_0) //tmp2
// sequence point: int tmp1 = (x > y) ? 1 : 0;
IL_0000: ldarg.0
IL_0001: ldarg.1
IL_0002: cgt
// sequence point: int tmp2 = (x < y) ? 1 : 0;
IL_0004: ldarg.0
IL_0005: ldarg.1
IL_0006: clt
IL_0008: stloc.0
// sequence point: return tmp1 - tmp2;
IL_0009: ldloc.0
IL_000a: sub
IL_000b: ret
}
""");
}

[Fact, WorkItem(61483, "https://github.com/dotnet/roslyn/issues/61483")]
public void Branchless_Operations()
{
var source = """
using static System.Console;

class C
{
static void M(int x, int y, bool b)
{
Write(x == y ? 1 : 0);
Write(x == y ? 0 : 1);
Write(x < y ? 1 : 0);
Write(x < y ? 0 : 1);
Write(x > y ? 1 : 0);
Write(x > y ? 0 : 1);
Write(b ? 0 : 1);
Write(!b ? 0 : 1);
Write(x <= y ? true : false);
Write(x <= y ? false : true);
}
}
""";
var verifier = CompileAndVerify(source);
verifier.VerifyDiagnostics();
verifier.VerifyMethodBody("C.M", """
{
// Code size 100 (0x64)
.maxstack 2
// sequence point: Write(x == y ? 1 : 0);
IL_0000: ldarg.0
IL_0001: ldarg.1
IL_0002: ceq
IL_0004: call "void System.Console.Write(int)"
// sequence point: Write(x == y ? 0 : 1);
IL_0009: ldarg.0
IL_000a: ldarg.1
IL_000b: ceq
IL_000d: ldc.i4.0
IL_000e: ceq
IL_0010: call "void System.Console.Write(int)"
// sequence point: Write(x < y ? 1 : 0);
IL_0015: ldarg.0
IL_0016: ldarg.1
IL_0017: clt
IL_0019: call "void System.Console.Write(int)"
// sequence point: Write(x < y ? 0 : 1);
IL_001e: ldarg.0
IL_001f: ldarg.1
IL_0020: clt
IL_0022: ldc.i4.0
IL_0023: ceq
IL_0025: call "void System.Console.Write(int)"
// sequence point: Write(x > y ? 1 : 0);
IL_002a: ldarg.0
IL_002b: ldarg.1
IL_002c: cgt
IL_002e: call "void System.Console.Write(int)"
// sequence point: Write(x > y ? 0 : 1);
IL_0033: ldarg.0
IL_0034: ldarg.1
IL_0035: cgt
IL_0037: ldc.i4.0
IL_0038: ceq
IL_003a: call "void System.Console.Write(int)"
// sequence point: Write(b ? 0 : 1);
IL_003f: ldarg.2
IL_0040: ldc.i4.0
IL_0041: ceq
IL_0043: call "void System.Console.Write(int)"
// sequence point: Write(!b ? 0 : 1);
IL_0048: ldarg.2
IL_0049: call "void System.Console.Write(int)"
// sequence point: Write(x <= y ? true : false);
IL_004e: ldarg.0
IL_004f: ldarg.1
IL_0050: cgt
IL_0052: ldc.i4.0
IL_0053: ceq
IL_0055: call "void System.Console.Write(bool)"
// sequence point: Write(x <= y ? false : true);
IL_005a: ldarg.0
IL_005b: ldarg.1
IL_005c: cgt
IL_005e: call "void System.Console.Write(bool)"
// sequence point: }
IL_0063: ret
}
""");
}

[Fact, WorkItem(638289, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/638289")]
public void ConditionalDelegateInterfaceUnification1()
{
Expand Down Expand Up @@ -1152,21 +1290,17 @@ static void Main()
comp.VerifyDiagnostics();
comp.VerifyIL("C.Main", @"
{
// Code size 25 (0x19)
.maxstack 2
// Code size 19 (0x13)
.maxstack 3
IL_0000: ldc.i4.1
IL_0001: dup
IL_0002: brtrue.s IL_0007
IL_0004: ldc.i4.1
IL_0005: br.s IL_0008
IL_0007: ldc.i4.0
IL_0008: call ""void System.Console.Write(int)""
IL_000d: brtrue.s IL_0012
IL_000f: ldc.i4.1
IL_0010: br.s IL_0013
IL_0012: ldc.i4.0
IL_0013: call ""void System.Console.Write(uint)""
IL_0018: ret
IL_0002: ldc.i4.0
IL_0003: ceq
IL_0005: call ""void System.Console.Write(int)""
IL_000a: ldc.i4.0
IL_000b: ceq
IL_000d: call ""void System.Console.Write(uint)""
IL_0012: ret
}");
}

Expand Down
48 changes: 23 additions & 25 deletions src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs
View file
Open in desktop
Expand Up @@ -5891,7 +5891,7 @@ public static void Main()
var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
compVerifier.VerifyIL("C.M1", @"
{
// Code size 38 (0x26)
// Code size 32 (0x20)
.maxstack 2
.locals init (bool V_0)
IL_0000: ldarg.0
Expand All @@ -5914,33 +5914,32 @@ .locals init (bool V_0)
IL_001c: ldc.i4.0
IL_001d: stloc.0
IL_001e: ldloc.0
IL_001f: brtrue.s IL_0024
IL_0021: ldc.i4.0
IL_0022: br.s IL_0025
IL_0024: ldc.i4.1
IL_0025: ret
IL_001f: ret
}
");
compVerifier.VerifyIL("C.M2", @"
{
// Code size 25 (0x19)
// Code size 30 (0x1e)
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
.maxstack 2
IL_0000: ldarg.0
IL_0001: ldc.i4.s 65
IL_0003: blt.s IL_000a
IL_0005: ldarg.0
IL_0006: ldc.i4.s 90
IL_0008: ble.s IL_0017
IL_0008: ble.s IL_001c
IL_000a: ldarg.0
IL_000b: ldc.i4.s 97
IL_000d: blt.s IL_0014
IL_000d: blt.s IL_0019
IL_000f: ldarg.0
IL_0010: ldc.i4.s 122
IL_0012: ble.s IL_0017
IL_0012: cgt
IL_0014: ldc.i4.0
IL_0015: br.s IL_0018
IL_0017: ldc.i4.1
IL_0018: ret
IL_0015: ceq
IL_0017: br.s IL_001a
IL_0019: ldc.i4.0
IL_001a: br.s IL_001d
IL_001c: ldc.i4.1
IL_001d: ret
}
");

Expand All @@ -5949,7 +5948,7 @@ .maxstack 2
compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
compVerifier.VerifyIL("C.M1", @"
{
// Code size 35 (0x23)
// Code size 30 (0x1e)
.maxstack 2
.locals init (bool V_0)
IL_0000: ldarg.0
Expand All @@ -5971,33 +5970,32 @@ .locals init (bool V_0)
IL_001a: ldc.i4.0
IL_001b: stloc.0
IL_001c: ldloc.0
IL_001d: brtrue.s IL_0021
IL_001f: ldc.i4.0
IL_0020: ret
IL_0021: ldc.i4.1
IL_0022: ret
IL_001d: ret
}
");
compVerifier.VerifyIL("C.M2", @"
{
// Code size 24 (0x18)
// Code size 28 (0x1c)
.maxstack 2
IL_0000: ldarg.0
IL_0001: ldc.i4.s 65
IL_0003: blt.s IL_000a
IL_0005: ldarg.0
IL_0006: ldc.i4.s 90
IL_0008: ble.s IL_0016
IL_0008: ble.s IL_001a
IL_000a: ldarg.0
IL_000b: ldc.i4.s 97
IL_000d: blt.s IL_0014
IL_000d: blt.s IL_0018
IL_000f: ldarg.0
IL_0010: ldc.i4.s 122
IL_0012: ble.s IL_0016
IL_0012: cgt
IL_0014: ldc.i4.0
IL_0015: ret
IL_0016: ldc.i4.1
IL_0015: ceq
IL_0017: ret
IL_0018: ldc.i4.0
IL_0019: ret
IL_001a: ldc.i4.1
IL_001b: ret
}
");
}
Copy link
Contributor Author

@jjonescz jjonescz Mar 8, 2023 •
edited

Choose a reason for hiding this comment

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

Correctness_Rebuild fails with weird errors (AzDo build).

  1. It crashes with OOM (repeatedly - can even reproduce that locally).

    Details: 
    2023-03-07T14:25:15.5083562Z Validation failed for D:\a\_work\1\s\artifacts/obj/Microsoft.CodeAnalysis.ExpressionCompiler\Release\netstandard2.0\Microsoft.CodeAnalysis.ExpressionEvaluator.ExpressionCompiler.dll
2023-03-07T14:25:15.5084335Z Writing diffs to "D:\a\_work\1\s\artifacts\BuildValidator\netstandard2.0\Microsoft.CodeAnalysis.ExpressionEvaluator.ExpressionCompiler"
2023-03-07T14:25:16.6559691Z Exception of type 'System.OutOfMemoryException' was thrown.
2023-03-07T14:25:17.2868546Z Unhandled exception: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.BadImageFormatException: Exception of type 'System.OutOfMemoryException' was thrown.
2023-03-07T14:25:17.2912239Z    at System.Reflection.PortableExecutable.PEReader.DecodeEmbeddedPortablePdbDebugDirectoryData(AbstractMemoryBlock block)
2023-03-07T14:25:17.2913380Z    at System.Reflection.PortableExecutable.PEReader.ReadEmbeddedPortablePdbDebugDirectoryData(DebugDirectoryEntry entry)
2023-03-07T14:25:17.2914522Z    at System.Reflection.PortableExecutable.PEReader.TryOpenEmbeddedPortablePdb(DebugDirectoryEntry embeddedPdbEntry, Boolean& openedEmbeddedPdb, MetadataReaderProvider& provider, Exception& errorToReport)
2023-03-07T14:25:17.2915533Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2916473Z    at System.Reflection.PortableExecutable.PEReader.TryOpenAssociatedPortablePdb(String peImagePath, Func`2 pdbFileStreamProvider, MetadataReaderProvider& pdbReaderProvider, String& pdbPath)
2023-03-07T14:25:17.2917705Z    at BuildValidator.Program.ValidateFile(Options options, AssemblyInfo assemblyInfo, ILogger logger, LocalReferenceResolver referenceResolver) in /_/src/Tools/BuildValidator/Program.cs:line 278
2023-03-07T14:25:17.2918862Z    at BuildValidator.Program.ValidateFiles(IEnumerable`1 assemblyInfos, Options options, ILoggerFactory loggerFactory) in /_/src/Tools/BuildValidator/Program.cs:line 203
2023-03-07T14:25:17.2920092Z    at BuildValidator.Program.HandleCommand(String[] assembliesPath, String[] exclude, String sourcePath, String[] referencesPath, Boolean verbose, Boolean quiet, Boolean debug, String debugPath) in /_/src/Tools/BuildValidator/Program.cs:line 140
2023-03-07T14:25:17.2921119Z    --- End of inner exception stack trace ---
2023-03-07T14:25:17.2921791Z    at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
2023-03-07T14:25:17.2922646Z    at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
2023-03-07T14:25:17.2923305Z    at System.Delegate.DynamicInvokeImpl(Object[] args)
2023-03-07T14:25:17.2923909Z    at System.CommandLine.Invocation.ModelBindingCommandHandler.<InvokeAsync>d__12.MoveNext()
2023-03-07T14:25:17.2924532Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2925172Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2925877Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2926703Z    at System.CommandLine.Invocation.InvocationPipeline.<>c__DisplayClass4_0.<<BuildInvocationChain>b__0>d.MoveNext()
2023-03-07T14:25:17.2927388Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2928302Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2928941Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2929653Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<UseParseErrorReporting>b__21_0>d.MoveNext()
2023-03-07T14:25:17.2930293Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2930858Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2931547Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2932274Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass16_0.<<UseHelp>b__0>d.MoveNext()
2023-03-07T14:25:17.2933007Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2933583Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2934255Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2935036Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass25_0.<<UseVersionOption>b__0>d.MoveNext()
2023-03-07T14:25:17.2935862Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2936465Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2937091Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2937850Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass23_0.<<UseTypoCorrections>b__0>d.MoveNext()
2023-03-07T14:25:17.2938507Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2939096Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2939726Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2940461Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<UseSuggestDirective>b__22_0>d.MoveNext()
2023-03-07T14:25:17.2941157Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2941704Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2942359Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2943033Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<UseParseDirective>b__20_0>d.MoveNext()
2023-03-07T14:25:17.2943675Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2944209Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2944873Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2945555Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<UseDebugDirective>b__11_0>d.MoveNext()
2023-03-07T14:25:17.2946187Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2946767Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2947395Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
2023-03-07T14:25:17.2948145Z    at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<RegisterWithDotnetSuggest>b__10_0>d.MoveNext()
2023-03-07T14:25:17.2948773Z --- End of stack trace from previous location where exception was thrown ---
2023-03-07T14:25:17.2949358Z    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
2023-03-07T14:25:17.2949983Z    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

  2. Before it crashes, BuildValidator.exe also reports different IL after rebuild in test classes - seems exactly the test classes that this PR touches. Constants are emitted differently, I have no idea why.

    Details:

    For example, in PatternSwitchTests.DuplicateDouble:

    -    IL_000a:  /* 23   | 40091EB851EB851F */ ldc.r8     3.1400000000000001
+    IL_000a:  /* 23   | 40091EB951EB851F */ ldc.r8     3.1400019073486329

    In AttributeTests.TestAttributesOnPropertyAndGetSet:

    @@ -2331,7 +2331,7 @@
       IL_00fc:  /* A2   |                  */ stelem.ref
       IL_00fd:  /* 25   |                  */ dup
       IL_00fe:  /* 19   |                  */ ldc.i4.3
-      IL_00ff:  /* 23   | 400921FB4D12D84A */ ldc.r8     3.1415926000000001
+      IL_00ff:  /* 23   | 400921FC4D12D84A */ ldc.r8     3.1415945073486329
       IL_0108:  /* 8C   | (01)0002D0       */ box        [mscorlib/*23000001*/]System.Double/*010002D0*/
     + "}\r\n1.0m !\r\nnull" /* 7036B889 */
-    IL_0036:  /* 23   | 4000CCCCCCCCCCCD */ ldc.r8     2.1000000000000001
+    IL_0036:  /* 23   | 4000CCCDCCCCCCCD */ ldc.r8     2.1000019073486329
     IL_003f:  /* 8C   | (01)0002D0       */ box        [mscorlib/*23000001*/]System.Double/*010002D0*/
     IL_0044:  /* 22   | 40066666         */ ldc.r4     2.0999999
     IL_0049:  /* 8C   | (01)0002D1       */ box        [mscorlib/*23000001*/]System.Single/*010002D1*/
@@ -595932,7 +595932,7 @@
     IL_0050:  /* 8D   | (01)000014       */ newarr     [mscorlib/*23000001*/]System.Object/*01000014*/
     IL_0055:  /* 25   |                  */ dup     IL_0056:  /* 16   |                  */ ldc.i4.0
-    IL_0057:  /* 23   | 3FF028F5C28F5C29 */ ldc.r8     1.01
+    IL_0057:  /* 23   | 3FF028F6C28F5C29 */ ldc.r8     1.0100009536743164
     IL_0060:  /* 0D   |                  */ stloc.3     IL_0061:  /* 12   | 03               */ ldloca.s   V_3
     IL_0063:  /* 28   | (0A)0005CA       */ call       instance string [mscorlib/*23000001*/]System.Double/*010002D0*/::ToString() /* 0A0005CA */

Copy link
Member

Choose a reason for hiding this comment

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

Let me try debugging that this morning and see if I come up with anything.

Copy link
Member

Choose a reason for hiding this comment

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

Okay I figured out the OOM issue. The OOM is coming from a call to AllocHGlobal deep inside the PEStream loading code. The problem with AllocHGlobal is that it's a native memory allocation function and hence doesn't represent any type of pressure to the GC. While we have memory available here, it's just not collected, there isn't enough to satisfy the 41MB request that happens because allocating native memory doesn't pressure the GC to do a collect.

I believe this is happening in your case because the earlier failures in the execution cause more memory to get loaded (to dump XML, MDV, etc ...). That pushes us over the limit and causes a crash.

Short term I fixed this by just making the app run as 64 bit locally. Still looking at the actual errors.

Copy link
Member

Choose a reason for hiding this comment

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

Pushed a test change to your PR and yes that fixed the issue. Will open a separate PR for that but will keep it here so it's not blocking you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for fixing the OOM. I see that the BuildValidator doesn't report any IL differences anymore - is that expected? I thought those two failures were unrelated, but maybe not.

Expand Down
22 changes: 9 additions & 13 deletions src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs
View file
Open in desktop
Expand Up @@ -11960,7 +11960,7 @@ .maxstack 1

diff1.VerifyIL("C.F", @"
{
// Code size 46 (0x2e)
// Code size 40 (0x28)
.maxstack 1
.locals init ([int] V_0,
[bool] V_1,
Expand All @@ -11980,20 +11980,16 @@ .maxstack 1
IL_0013: ldc.i4.0
IL_0014: stloc.s V_4
IL_0016: ldloc.s V_4
IL_0018: brfalse.s IL_0026
IL_0018: brfalse.s IL_0020
IL_001a: nop
IL_001b: ldloc.3
IL_001c: brtrue.s IL_0021
IL_001e: ldc.i4.0
IL_001f: br.s IL_0022
IL_0021: ldc.i4.1
IL_0022: stloc.s V_5
IL_0024: br.s IL_002b
IL_0026: ldc.i4.0
IL_0027: stloc.s V_5
IL_0029: br.s IL_002b
IL_002b: ldloc.s V_5
IL_002d: ret
IL_001c: stloc.s V_5
IL_001e: br.s IL_0025
IL_0020: ldc.i4.0
IL_0021: stloc.s V_5
IL_0023: br.s IL_0025
IL_0025: ldloc.s V_5
IL_0027: ret
}");

var diff2 = compilation2.EmitDifference(
Expand Down