Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Treat BitConverter.IsLittleEndian as an intrinsic #9789

Merged
merged 1 commit into from
Feb 26, 2017

Conversation

mikedn
Copy link

@mikedn mikedn commented Feb 25, 2017

Fixes #9701

@stephentoub
Copy link
Member

stephentoub commented Feb 25, 2017

This could of course be changed again, but currently the BitConverter that's used by all code outside of corelib is the one defined in CoreFx:
https://github.com/dotnet/corefx/blob/master/src/System.Runtime.Extensions/src/System/BitConverter.cs
in System.Runtime.Extensions.dll. Will this change treat its IsLittleEndian as an intrinsic? Looks like it's specific to the one in mscorlib.

@mikedn
Copy link
Author

mikedn commented Feb 25, 2017

@stephentoub See the discussion at the end of #9701 and #9788

@stephentoub
Copy link
Member

I see, thanks.

@jkotas
Copy link
Member

jkotas commented Feb 25, 2017

@jashook @prajwal-aithal @sjsinju Linux ARM Emulator builds are failing with:

ERROR: Unable to find project for artifact copy: dotnet_corefx/master/linuxarmemulator_softfp_cross_debug

Could you please take a look?

@hqueue
Copy link
Member

hqueue commented Feb 25, 2017

@jkotas It's because corefx CI change(dotnet/corefx#16378) has been merged. It can be fixed after #9445 is merged.

@hqueue
Copy link
Member

hqueue commented Feb 25, 2017

@jkotas I made a quick fix for arm CI break #9797 from #9445.

@hqueue
Copy link
Member

hqueue commented Feb 25, 2017

@dotnet-bot test Linux ARM Emulator Cross Release Build
@dotnet-bot test Linux ARM Emulator Cross Debug Build

@jkotas
Copy link
Member

jkotas commented Feb 26, 2017

@dotnet/jit-contrib Could you please take a look?

Copy link

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

LGTM

@pgavlin pgavlin merged commit c5e2822 into dotnet:master Feb 26, 2017
@AndyAyersMS
Copy link
Member

Do we have a writeup anywhere on what changes should require jit GUID changes?

Adding a new jit intrinsic could generally fall in the "does not require" camp since an older release jit will fail to recognize the new intrinsics and will generate a call instead, but it looks like older debug/checked jits will assert.

@mikedn
Copy link
Author

mikedn commented Feb 26, 2017

an older release jit will fail to recognize the new intrinsics and will generate a call instead

But how would an older jit end up being used with a new runtime?

@AndyAyersMS
Copy link
Member

To be clear: using a mismatched jit/runtime is not an officially supported scenario.

However we do this sort of thing somewhat often during development and bug triage, when making A/B comparisons of jit behavior, or bisecting to track down the origin of bugs.

For instance say somebody reports a bug in the CurrentCLR build -- if we suspect it is a jit regression it is often helpful to quickly pin down when and where the bug was introduced, say to revert a particular change (such things can even be automated). Doing this with the whole CoreCLR package can be awkward or even impossible, if non-jit aspects have changed; the scenario may not even build/run on older runtimes. But the jit/runtime interface typically changes more slowly, so it is often feasible to bisect by just varying jits -- and the GUID changes are one way to mark the plausible ranges over where this can work. So while bisecting, we'swap in a succession of older jits into the newer runtime, and typically these are checked jit builds so we have the full benefit of dumping and internal asserts and diagnostics.

@mikedn mikedn deleted the le-intrinsic branch April 10, 2017 05:47
RussKeldorph added a commit that referenced this pull request Apr 26, 2017
Maintain compat with JIT32 for 2.0 Preview
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Treat BitConverter.IsLittleEndian as an intrinsic

Commit migrated from dotnet/coreclr@c5e2822
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants