Some IL tests are failing due to the change.

There are some build errors on my machine (it seems to be new F# features like [x..] indexing... How can I build this locally ?

Some IL tests are failing due to the change.

There are some build errors on my machine (it seems to be new F# features like [x..] indexing... How can I build this locally ?

Build scripts (from the devguide) should work just fine. You can use it with noVisualStudio switch.

Devguide also describes how to update IL baselines (if test uses baseline files).

I'm curious if we know that this is the fastest implementation. Is there any information on this on the web e.g. for C++ or assembly code?

I'd like to see a systematic correctness test matrix for comparisons on all the basic types affected by this, including

• MaxValue
• MaxValue - 1
• MinValue
• MinValue + 1
• 0
• 1
• -1

Also don't forget the char and bool types

There could be some improvements in the jit.
For instance, the cmp is done twice.
Since setgt setlt don't modify flags, both could be done with a single cmp.
And the double zero extension could be done after the sub as a sign extension.

I think the fastest version would be in x64
cmp eax ecx
setg al
setl dl
sub dl, al
movsx eax, dl

There is a 4 instr version found on SO

sub %1, %0
jno 1f
cmc
rcr %0
https://stackoverflow.com/questions/10996418/efficient-integer-compare-function

But it involve a conditional jump that will be slower on branch prediction. Moreover I see no way to make the jit compile something like this.

I was also looking at movcc (conditional move) instructions, but the source is a register or memory, it cannot be a constant. So it requires extra instructions to load constants and won't we shorter.
That could be interesting to implement min and max for instance.

After a check, the requirement on IComparable.CompareTo is:

• when x > y, result is > 0
• when x < y, result is < 0
• when x=y result is 0

for shorter types (byte, short, char) it is implemented as:

int x - int y

This is valid since there is no risk of overflow. But the result is not necessarily 0,1,-1

For longer types, it uses conditional jumps and returns always 0,1,-1

Do we have that constraint on the compare function ?

Since compare fallbacks to ICompare.Compare for other types, there is no guarantee that the output is always 0,1,-1.

https://docs.microsoft.com/en-us/dotnet/api/system.collections.icomparer.compare?view=net-6.0#system-collections-icomparer-compare(system-object-system-object)

This would not be a breaking change at the specification level, but it would be a breaking change at the implementation level...

code like:
if compare x y = 1 then ... would be broken.

Same thing for code like:
ls |> List.sum (fun (x,y) -> compare x y)

But such code is not following the specification of compare.

There are a few tests that compare generated IL to expected IL that fail due to new code generation.

How do you usually regenerate expected IL ?

There are a few tests that compare generated IL to expected IL that fail due to new code generation.

How do you usually regenerate expected IL ?

So, if they are inline (i.e. the IL itself is the string in the test case itself, then, unfortunately, only one-by-one).
If they're verified as part of DirectoryAttribute-style testing, then, an ENV variable can be set, and then run tests locally:|

// on Windows
set TEST_UPDATE_BSL=1
// or, on macOS/Linux
export TEST_UPDATE_BSL=1
// and then
build.cmd -testCoreClr
// or
build.sh --testcoreclr

(https://github.com/dotnet/fsharp/blob/main/DEVGUIDE.md#updating-baselines-in-tests)

There are a lot of culture dependent tests that fail on my french machine . Fixed it.
There was also a lot of over specific autogenerated tests for compare. They were checking for the exact response of compare... The check must be done on the sign, not the exact value. Fixed it.

Some of the builds fail but it doesn't seem related to the change.. Any idea?

Some of the builds fail but it doesn't seem related to the change.. Any idea?

The latest run failed because of the tests:
https://dev.azure.com/dnceng/public/_build/results?buildId=1791081&view=ms.vss-test-web.build-test-results-tab&runId=47885210&resultId=100293&paneView=debug

These are baseline tests, but they don't seem to be updated with the instructions above...

I've updated them + updated the guide, If I may ask, which shell were you using?

Great, now it's even more failures...I will take a look.

I was using powershell so I used
$env:TEST_UPDATE_BSL=1

I was using powershell so I used
$env:TEST_UPDATE_BSL=1

Hm, interesting. I will probably add a separate switch to the build scripts which runs tests and updates baselines.

@thinkbeforecoding I have updated 2 remaining baselines for net472 framework + updated devguide describing how it works.
To update them successfully, It requires some knowledge of how IL baselines work. I will add a separate command to build scripts for updating those (#13204)

The rest of the tests which are failing are checking the compare result.

@thinkbeforecoding I have updated 2 remaining baselines for net472 framework + updated devguide describing how it works. To update them successfully, It requires some knowledge of how IL baselines work. I will add a separate command to build scripts for updating those (#13204)

The rest of the tests which are failing are checking the compare result.

computers are harsh:
System.Exception : args({ Item = 0us }, { Item = 65535us }) Expected=-1. Received=-65535.
0 - 65535 is indeed -65535 and not the -1 the program was expecting.

Since compare fallbacks to ICompare.Compare for other types, there is no guarantee that the output is always 0,1,-1.

I don't know about the no guarantee thing. If someone upgrades their dotnet sdk recompiles their app, and their original source code now has a "hard to analyze bug" pretty sure we would revert the change whilst looking red faced when saying yes we knew it could break people, but we thought it was rare enough to risk. And amusingly it was even caught by tests, which is an eye opener.

Actually, byte.CompareTo or char.CompareTo return a different result from F# compare. Some types return only 0,-1,1 others return a large range of values.

Yet I can understand the impact at large scale.
However, the Bcl decided to randomize GetHashCode which was a similar breaking change. The spec has always specified not to rely on it value, but it probably happened... Was there bad feedback?

Would it be possible to add an analyser or a warning for comparison of the result of compare to anything other than 0?

For now, progress is hindered by #13212 Running test suites locally is problematic on non en-US machines.

When I used the command it said:
Commenter does not have sufficient privileges for PR 13187 in repo dotnet/fsharp

There are still some cancelled builds... strange

These canceled builds again.

These canceled builds again.

Yeah, some infra hiccups, I suppose.

@dsyme , I think this just needs your review, but should be good.

@dsyme, this is good to go, do you want to review your change request.

@thinkbeforecoding Thank you so much for this improvement!

Wohooo! It landed. Awesome work!

NOTE: as of .NET 7.0 (or was it 8.0), JIT is expected to use branchless instructions even when F#/C# emits branches (recognizes cmov like idioms)

Yes I've noted this in some tests in fasmi. I think it's in dotnet 8.0

Sure, I just wanted to note that it's better when some sub-optimal codegen is filed against JIT rather than silently (for JIT team) fixed 🙂