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.

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

JIT: "default(T) == null" does not emit a constant when T is Nullable<> #34906

Closed
michkot opened this issue Apr 13, 2020 · 8 comments
Closed
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner

Comments

@michkot
Copy link

michkot commented Apr 13, 2020

After toying with efficient detection of reference types / value types / nullable types I triggered a discovery of generic context issue #34641.

I now also find out that default(T) == null for T being Nullable<XXX> in the following snippet does not produce a constant in ASM but rather builds what seems to be a composed "struct + HasValue bool" on stack and then cmps to 0x0.
sharplab.io link with longer example

public static class C<TVal>  {
  public static bool Test(TVal val)
  {
     if (default(TVal) == null)
     {
        return true;
     }
     return false;
  }
}

The produced assembly is (I guess) valid, although quite inefficient. (And I did not find other good way to determine whether type is reference of a non-nullable valuetype.)

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Apr 13, 2020
@EgorBo
Copy link
Member

EgorBo commented Apr 13, 2020

I now also find out that default(T) == null for T being Nullable in the following snippet does not produce a constant in ASM

why should it produce a constant? a variable of a Nullable type can be null (or not)

@khellang
Copy link
Member

@michkot Have you seen #1157 and #32098?

@benaadams
Copy link
Member

why should it produce a constant? a variable of a Nullable type can be null (or not)

default of a nullable is always null so its constant?

e.g. default(int?) is always null

@EgorBo
Copy link
Member

EgorBo commented Apr 13, 2020

why should it produce a constant? a variable of a Nullable type can be null (or not)

default of a nullable is always null so its constant?

e.g. default(int?) is always null

Ah, oops.

@EgorBo
Copy link
Member

EgorBo commented Apr 13, 2020

@benaadams @michkot anyway I can't repro it on my master, probably is already fixed (e.g. here? #32269)

It's not the first time people use sharplab with .NET Core 3.0 to file issues against RyuJIT-master 😐

@michkot
Copy link
Author

michkot commented Apr 13, 2020

@benaadams @michkot anyway I can't repro it on my master, probably is already fixed (e.g. here? #32269)

It's not the first time people use sharplab with .NET Core 3.0 to file issues against RyuJIT-master 😐

Thanks for looking into it. I will close the issue now then 👍 . Can you point me at some resources/docs so I can try it myself at current master @EgorBo ? (I had luck the last time with sharplab, so I tried it again).

@michkot michkot closed this as completed Apr 13, 2020
@michkot
Copy link
Author

michkot commented Apr 13, 2020

@michkot Have you seen #1157 and #32098?

I did not explicitly, but I noticted that work being done in other issue. That feature/opt. is also nice when you need reference/value type distinction. I trully care about null being a default here though :)

@EgorBo
Copy link
Member

EgorBo commented Apr 13, 2020

@michkot
my workflow is this:

git clone git@github.com:dotnet/runtime.git
cd runtime/src/coreclr
./build.sh -checked

once it finishes:

cd myConsoleApp
dotnet publish -c Release -r osx-x64
cp ~/runtime/artifacts/bin/coreclr/OSX.x64.Checked/* ~/myConsoleApp/bin/netcoreapp3.1/osx-x64/publish
cd ~/myConsoleApp/bin/netcoreapp3.1/osx-x64/publish
COMPlus_JitDisasm=MyNamespace.MyProgram:MyMethod ./myConsoleApp

Also, let me promote my add-in for that: https://github.com/EgorBo/Disasmo (it still needs a pre-built runtime in Checked config)

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

5 participants