[IL Emit] Impove string null check il codegen

[En3Tho]

Consider those 2 methods:

let test() =
    while Console.ReadLine() <> null do
        Console.WriteLine(1)

static void Test()
{
    while (Console.ReadLine() != null)
        Console.WriteLine(1);
}

If we compare outputs of F# and C# compilers in IL we can see that C# does a simple nullpointer check while F# calls to String.Equals method

F# version:

// loop start
            IL_0000: call string [System.Console]System.Console::ReadLine()
            IL_0005: ldnull
            IL_0006: call bool [netstandard]System.String::Equals(string, string)
            IL_000b: brtrue.s IL_0015 // F# is emitting a String.Equals call which then will be cut by jit in runtime but it increases dll size nonetheless

            IL_000d: ldc.i4.1
            IL_000e: call void [System.Console]System.Console::WriteLine(int32)
            IL_0013: br.s IL_0000
        // end loop

        IL_0015: ret

C# version:

// sequence point: hidden
        IL_0000: br.s IL_0008
        // loop start (head: IL_0008)
            IL_0002: ldc.i4.1
            IL_0003: call void [System.Console]System.Console::WriteLine(int32)

            IL_0008: call string [System.Console]System.Console::ReadLine()
            IL_000d: brtrue.s IL_0002 // C# is emitting a simply nullpointer check here
        // end loop

        IL_000f: ret

Note that C# on top of this reverses condition and useful work resulting in better jit codegen. Coincidentally this is even more simplified repro for (https://github.com/dotnet/fsharp/issues/12138).

[EgorBo]

Yeah, replacing String.op_Equality(str1, null) to str1 ceq null makes output IL smaller and makes jit's life easier during inlining.

[dsyme]

@En3Tho Could you include a micro-benchmark perf result or x64 ASM code?

[En3Tho]

Method	Job	Runtime	value	Mean	Error	StdDev	Median	Code Size	Allocated
CompareStringViaEquals	.NET 5.0	.NET 5.0		1.4297 ns	0.0044 ns	0.0034 ns	1.4299 ns	63 B	-
CompareStringViaRefEquals	.NET 5.0	.NET 5.0		0.0077 ns	0.0105 ns	0.0098 ns	0.0024 ns	10 B	-

[En3Tho]

[<DisassemblyDiagnoser>]
[<MemoryDiagnoser>]
[<SimpleJob(RuntimeMoniker.Net50)>]
[<SimpleJob(RuntimeMoniker.Net60)>]
type Benchmark() =
    [<Benchmark>]
    [<Arguments("")>]
    member _.CompareStringViaEquals(value: string) = value = null

    [<Benchmark>]
    [<Arguments("")>]
    member _.CompareStringViaRefEquals(value: string) = Object.ReferenceEquals(value, null)

[En3Tho]

I thought that JIT could recognize this pattern and this is IL-only problem but sadly, no.

[kerams]

= null is not good practice. What about isNull though?

https://latkin.org/blog/2015/05/18/null-checking-considerations-in-f-its-harder-than-you-think/

[En3Tho]

It's a perfectly good practice for anybody coming from another language. For example, C# has string == null. A person would expect the very similar thing to work in F#. And if it's a bad one, F# should have an analyzer for this. Althought I don't really see why it's a bad practice honestly. Somethimes you need to check for null only, not for NullOrEmpty or Whatespace or whatever