search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.86k stars 4.62k forks source link

JIT: review properties of jit helpers #7723

Open AndyAyersMS opened 7 years ago

AndyAyersMS commented 7 years ago

Some helpers are missing explicit entries in the helper classifier code at HelperCallProperties::init(). Seems like we should adopt a policy where all helper properties are spelled out explicitly and the default case asserts, so when a new helper is added we know we should go add the properties.

For instance CORINFO_HELP_READYTORUN_GENERIC_HANDLE is not listed and calls to it likely can be removed if the result is unused.

category:cq theme:helpers skill-level:expert cost:medium

AndyAyersMS commented 6 years ago

CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE won't return null. The MAYBE_NULL variant will but isn't listed at all (perhaps not a big deal as it seems to only be used for TypedReference which is rare).

Also have seen several cases where we null check the result of a NEW helper, despite it being marked as returning an non-null value. Will try and update this issue with a specific example.

So perhaps there are cases where some properties aren't taken into account?

AndyAyersMS commented 6 years ago

There's an example of null check after new in dotnet/coreclr#14472 -- the new is from an inlined box.

AndyAyersMS commented 6 years ago

Considering this for 2.1, so changing milestone.

AndyAyersMS commented 6 years ago

Probably not happening in time for 2.1, so pushing it out to Future once more.

TIHan commented 10 months ago

Keeping this as future unless we believe we should review this for .NET 9.