See if this helps. Windows x64-centric, but steps are fairly similar on other arches or on Linux (and/or you can use an altjit).

cc @dotnet/jit-contrib -- we should have something like this in a more official place.

running jit diffs

Steps are

1. obtain jitutils and bootstrap
2. build base runtime and (optionally) build tests
3. build diff runtime
4. run jit-diffs

Presumes you have two clones of the runtime repo: base (on master) and diff (branched from same commit as master with only jit changes). You probably can get by with one runtime repo clone viagit worktree; I haven't tried this.

Steps 1 and 2 don't need to be rerun when you are making incremental jit changes in the diff repo; just redo steps 3 and 4 to get new diffs.

If you are also altering framework or vm code in your diff clone, things are more complex; in step 4 you must run base and diff independently, each in its respective core_root, and then runjit-analyze manually on the two result directories.

1. obtain tools

git clone https://github.com/dotnet/jitutilscd \repos\jitutilsbootstrap.cmd

2. build base repo runtime

cd \repos\base-runtimebuild -c release

Then build the test environment only

cd src\coreclrbuild-test.cmd x64 release skipmanaged skipnative

or optionally build the test environment and all the coreclr pri0 tests

build-test.cmd x64 release

or optionally build the test environment and all the coreclr pri0 and pri1 tests

build-test.cmd x64 release -priority 1

Then build a checked runtime and test environment

build.cmd x64 checked skiptests skipbuildpackagesbuild-test.cmd x64 checked skipmanaged skipnative

3. build diff repo runtime

First time:

cd \repos\diff-runtimebuild -c Releasecd src\coreclrbuild.cmd x64 checked skiptests skipbuildpackagesbuild-test.cmd x64 checked skipmanaged skipnative

Subsequently (when you've just changed jit files), you can skip the release build:

cd \repos\new-runtime\src\coreclrbuild.cmd x64 checked skiptests skipbuildpackagesbuild-test.cmd x64 checked skipmanaged skipnative

4. run jit-dffs

This is what I commonly run (PMI diffs across all FX).

jit-diff diff --pmi --base --base_root \repos\base-runtime --diff --diff_root \repos\diff-runtime  --core_root \repos\base-runtime\artifacts\tests\coreclr\Windows_NT.x64.Release\Tests\Core_Root -f

We should fix jit-diff so it can find core_root on its own.

You can also use a checked runtime as the core_root, but it's considerably slower and you will see debug assert codegen in SPC.

For other options try

jit-diff diff --help

For instance, to look at diffs in a particular assembly (say test cases you've created):

jit-diff diff --pmi  --base --base_root \repos\base-runtime   --diff --diff_root \repos\diff-runtime   --core_root \repos\base-runtime\artifacts\tests\coreclr\Windows_NT.x64.Release\Tests\Core_Root   --assembly <path to your assembly>

If you built tests in step 2 above, you can also diff over the tests, or just the benchmark subset. Diffing over all tests is sometimes a bit noisy, there are "expected failures", but they should be common to base and diff.

I ended up following the steps mentioned by@EgorBo in:
https://gist.github.com/EgorBo/044453d74d2a759113d3bff2f5f7e348

What I get is:

$~/projects/public/jitutils/bin/jit-analyze -b /tmp/jitdiff/dasmset_1/base -d /tmp/jitdiff/dasmset_2/base -r -c 15Found 5 files with textual diffs.Summary of Code Size diffs:(Lower is better)Total bytes of diff: 0 (0.000% of base)0 total files with Code Size differences (0 improved, 0 regressed), 161 unchanged.0 total methods with Code Size differences (0 improved, 0 regressed), 291049 unchanged.5 files had text diffs but no metric diffs.System.Private.CoreLib.dasm had 2510 diffsSystem.Private.Uri.dasm had 140 diffsSystem.Diagnostics.Process.dasm had 70 diffsSystem.Private.Xml.Linq.dasm had 58 diffsSystem.Management.dasm had 4 diffs

Is that the sort of output you accept/expect?

I can also follow the instructions by@AndyAyersMS (I will do it anyway for practice).

Also, thefailing check isn't making a lot of sense to me.
Is there someone who can tell me if/how this is directly related to the PR itself?

Yes, that's the kind of output we're looking for. It tells us there likely aren't any examples of this in the framework assemblies.

So you might consider adding a new test, or adding an example of this to some existing test.

Seems likeNOT(NOT(x)) should get similar treatment?

Seems likeNOT(NOT(x)) should get similar treatment?

Isn't it useful to normalize booleans (e.g. from pinvokes) to 0..1 ?

However, it seemsRoslyn already optimizesNOT(NOT(x)) so the normalization trick doesn't work

bytepinvoke_bool=2;// e.g. a bool from a pinvokebooltmp=*(bool*)&pinvoke_bool;tmp=!!tmp;// normalizeConsole.WriteLine(*((byte*)&tmp));// stil prints 2

See anothertrick;

Yes, that's the kind of output we're looking for. It tells us there likely aren't any examples of this in the framework assemblies.

So you might consider adding a new test, or adding an example of this to some existing test.

Seems likeNOT(NOT(x)) should get similar treatment?

As for adding a specific test, sure I'll give it a try.

I assume it's OK if the test is part of the PR and then doesn't appear in the JIT diff?

Kind of new to this, trying to wrap my head as to what is considered the standard procedure here.

You can run jit diffs over all the tests but since we usually use the "before" runtime repo as the baseline, this won't show diffs in your newly added tests. But you may discover cases in already existing tests.

I usually just run diffs on new tests in isolation, via the--assembly arg tojit-diff (see last bit of my note above).

seems Roslyn already optimizes NOT(NOT(x))

You can separate theNOTs by an inline boundary to keep Roslyn from messing with them.

@damageboy, are you still working on this?

@damageboy, are you still working on this?

Well, last I left this, there was little to do, but I forgot to push it in terms of discussion and pinging people.

I can drop this, or try to restart for 6.0

@AndyAyersMS, how should@damageboy proceed? Thanks.

I noticed that this pattern sometimes is used to blame jit for lack of peephole optimizations 😄 so would be nice to merge especially because the change is just a few lines of code

The change itself looks fine, so please do restart.

You will need to move the new test undersrc\tests.

I've rebased everything but still having some build issues with the tests per-se.

I'll give it another try on a different machine.

The failing test on AZP seem... unrelated? How should I proceed?

Also, since Roslyn optimizes double NOT I'd allow it in JIT too, something like this:

caseGT_NOT:caseGT_NEG:// Remove double negationif (op1->OperIs(oper))            {returnop1->AsOp()->gtGetOp1();            }/* Any constant cases should have been folded earlier */noway_assert(!op1->OperIsConst()|| !opts.OptEnabled(CLFLG_CONSTANTFOLD)||optValnumCSE_phase);break;

Thanks for the comments,@EgorBo.

I think the current state of affairs addresses your feedback.

Hi, I think I'm done with this.
I had this working (including in the CI) with no errors, but then I added another test, and the next CI run seems to be mostly OK except for a single CI pipeline that timed out without failing any test.

Let me know what you want me to do with this...?

@AndyAyersMS: I've implemented the same logic for GT_NOT, but Roslyn is currently isn't generating MSIL that would triggerGT_NOT:

[MethodImpl(MethodImplOptions.AggressiveOptimization|MethodImplOptions.AggressiveInlining)]staticboolNegationBool(boolvalue)=>!value;

  .method private hidebysig static bool  NegationBool(bool 'value') cil managed aggressiveinlining aggressiveoptimization  {    // Code size       5 (0x5)    .maxstack  8    IL_0000:  ldarg.0    IL_0001:  ldc.i4.0    IL_0002:  ceq    IL_0004:  ret  } // end of method doublenegate::NegationBool

How do you suggest I proceed?

an MSIL based test?
Is there a different secret handshake?

Here's a simple test case:

usingSystem;usingSystem.Runtime.CompilerServices;classX{staticintNot(intx)=>~x;[MethodImpl(MethodImplOptions.NoInlining)]staticintNotNot(intx)=>Not(Not(x));publicstaticintMain(){returnNotNot(100);}}

we currently produce

; Assembly listing for method X:NotNot(int):int; Emitting BLENDED_CODE for X64 CPU with AVX - Windows; optimized code; rsp based frame; partially interruptible; Final local variable assignments;;  V00 arg0         [V00,T00] (  3,  3   )     int  ->  rcx;# V01 OutArgs      [V01    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]   "OutgoingArgSpace";* V02 tmp1         [V02    ] (  0,  0   )     int  ->  zero-ref    "Inlining Arg";; Lcl frame size = 0G_M60739_IG01:                                                ;; bbWeight=1    PerfScore 0.00G_M60739_IG02:       8BC1moveax,ecx       F7D0noteax       F7D0noteax                                                ;; bbWeight=1    PerfScore 0.75G_M60739_IG03:       C3ret                                                ;; bbWeight=1    PerfScore 1.00

@AndyAyersMS Thanks, I've tested this now locally, and I see the code triggered (e.g. during breakpoint) and working as expected, as well as JitDumps for the new test case in thedoublenegate.cs file working as expected.

For the test code:

staticboolTest3(){varx=DoubleNotInt(6);vary=DoubleNotInt(DoubleNotInt(DoubleNotInt(x)));varz=DoubleNotInt(_dummyValueInt);if(x==6&&y==6&&z==6)returntrue;returnfalse;}[MethodImpl(MethodImplOptions.AggressiveOptimization|MethodImplOptions.AggressiveInlining)]staticintNotInt(intvalue)=>~value;[MethodImpl(MethodImplOptions.AggressiveOptimization|MethodImplOptions.AggressiveInlining)]staticintDoubleNotInt(intvalue)=>NotInt(NotInt(value));

I now see the following as part of the new JitDump:

*************** After end code gen, before unwindEmit()G_M3856_IG01:        ; func=00, offs=000000H, size=0004H, bbWeight=1    PerfScore 1.25, gcVars= {}, gcrefRegs=00000000 {}, byrefRegs=00000000 {}, gcvars, byref, nogc <-- Prolog IGIN0007: 000000 push     rbpIN0008: 000001 mov      rbp, rspG_M3856_IG02:        ; offs=000004H, size=0011H, bbWeight=1    PerfScore 3.50, gcrefRegs=00000000 {}, byrefRegs=00000000 {}, byref, iszIN0001: 000004 mov      rax, 0x7FA847B935E8IN0002: 00000E mov      eax, dword ptr [rax]IN0003: 000010 cmp      eax, 6IN0004: 000013 jne      SHORT G_M3856_IG05G_M3856_IG03:        ; offs=000015H, size=0005H, bbWeight=0.50 PerfScore 0.12, gcrefRegs=00000000 {}, byrefRegs=00000000 {}, byrefIN0005: 000015 mov      eax, 1G_M3856_IG04:        ; offs=00001AH, size=0002H, bbWeight=0.50 PerfScore 0.75, epilog, nogc, extendIN0009: 00001A pop      rbpIN000a: 00001B ret      G_M3856_IG05:        ; offs=00001CH, size=0002H, bbWeight=0.50 PerfScore 0.12, gcVars= {}, gcrefRegs=00000000 {}, byrefRegs=00000000 {}, gcvars, byrefIN0006: 00001C xor      eax, eaxG_M3856_IG06:        ; offs=00001EH, size=0002H, bbWeight=0.50 PerfScore 0.75, epilog, nogc, extendIN000b: 00001E pop      rbpIN000c: 00001F ret

Is there anything else you would like to see that you feel is missing?

Thanks, this looks good.

We've just made a bunch of automation investments to simplify some aspects of running diffs, so you are welcome to try using those (you'd need to merge your local branch up to the tip and possibly rebuild the jit).

Running diffs should now be as simple as running

python .\src\coreclr\scripts\superpmi.py asmdiffs

from the root of the repo.

I don't expect any diffs from your changes above, but you never know.

Going to restart the failed test legs... not sure what is going on. Looks like lots of timeouts.

Wondering if these failures are actually related. We're doing this optimization even for debug codegen which is probably not a great idea. You should addopts.OptimizationEnabled() as one of the conditions, eg

if ((oper == GT_NEG) && op1->OperIs(GT_NEG) && opts.OptimizationEnabled())

also we know alreadyoper == GT_NEG so you can drop that bit out.

if (op1->OperIs(GT_NEG) && opts.OptimizationEnabled())

and perhaps follow Egor's suggestion and fold the NOT/NEG cases together, via

if (op1->OperIs(oper) && opts.OptimizationEnabled())

Thanks, this looks good.

We've just made a bunch of automation investments to simplify some aspects of running diffs, so you are welcome to try using those (you'd need to merge your local branch up to the tip and possibly rebuild the jit).

Running diffs should now be as simple as running

python .\src\coreclr\scripts\superpmi.py asmdiffs

from the root of the repo.

I don't expect any diffs from your changes above, but you never know.

Will do and report here.

Wondering if these failures are actually related. We're doing this optimization even for debug codegen which is probably not a great idea. You should addopts.OptimizationEnabled() as one of the conditions, eg

if ((oper == GT_NEG) && op1->OperIs(GT_NEG) && opts.OptimizationEnabled())

also we know alreadyoper == GT_NEG so you can drop that bit out.

if (op1->OperIs(GT_NEG) && opts.OptimizationEnabled())

and perhaps follow Egor's suggestion and fold the NOT/NEG cases together, via

if (op1->OperIs(oper) && opts.OptimizationEnabled())

Sounds good, will do.

Latest looks good.

I'd encourage you in the future to just let the PR accumulate commits. We require squash on merge so all this is just for PR purposes -- I find seeing the incremental progress helps me review things better.

Latest looks good.

I'd encourage you in the future to just let the PR accumulate commits. We require squash on merge so all this is just for PR purposes -- I find seeing the incremental progress helps me review things better.

Sure.
Still having issues getting the tests to run on Linux/Ubuntu 20.04, weird filesystem time modification test fails.

I'll also try to get the python superpmi.py thingy going, no such luck for now.

I'll also try to get the python superpmi.py thingy going, no such luck for now.

Since the changes look good I'm going to merge.

Keep us updated on how it's going with local testing and/or using the new spmi scripts; there may be some rough edges there we need to address.