JIT: Fold "cmp & 1" to "cmp"

[EgorBo]

I noticed this pattern after loop clonning (it produces it for the condition for two loops), e.g.:

static void Test(int[] arr)
{
    int len = arr.Length;
    for (int i = 0; i < len; i++)
        arr[i] = 0;
}

Codegen diff: https://www.diffchecker.com/fdGIQqQJ

(Obviously, for this specific method the clonned loop should either be removed or not emitted in the first place but that is a different story)

SuperPMI diffs:

asm.aspnet.run.windows.x64.checked.1

Total bytes of base: 4071
Total bytes of diff: 3965
Total bytes of delta: -106 (-2.60% of base)
    diff is an improvement.

asm.benchmarks.run.windows.x64.checked.1

Total bytes of base: 34862
Total bytes of diff: 34379
Total bytes of delta: -483 (-1.39% of base)
    diff is an improvement.

asm.libraries.crossgen.windows.x64.checked.5

Total bytes of base: 64797
Total bytes of diff: 63798
Total bytes of delta: -999 (-1.54% of base)
    diff is an improvement.

asm.libraries.crossgen2.windows.x64.checked.3

Total bytes of base: 44678
Total bytes of diff: 43939
Total bytes of delta: -739 (-1.65% of base)
    diff is an improvement.

asm.libraries.pmi.windows.x64.checked.1

Total bytes of base: 75612
Total bytes of diff: 74511
Total bytes of delta: -1101 (-1.46% of base)
    diff is an improvement.

asm.tests.pmi.windows.x64.checked.1

Total bytes of base: 43352
Total bytes of diff: 42533
Total bytes of delta: -819 (-1.89% of base)
    diff is an improvement.

asm.tests_libraries.pmi.windows.x64.checked.1

Total bytes of base: 103704
Total bytes of diff: 102144
Total bytes of delta: -1560 (-1.50% of base)
    diff is an improvement.

[EgorBo]

@dotnet/jit-contrib @BruceForstall PTAL

[kunalspathak]

LGTM.

[sandreenko]

sorry, I am late for the review, two questions:

1. do we want to do it when optimizations are disabled?
2. Can op2 or tree be a CSE candidates that we destroy while CSE is keeping links on them?

[EgorBo]

1. I found plenty of such small peepholes in morph without OptimizationEnabled guard so it wasn't clear to me when we should really apply it - I guess we want to eventually have three different modes: MinOpts, tier0 (simple opts) and tier1 (all opts).

2. op2 is always an integer const (1) and CSE doesn't work for them without a special switch -- so it's not a correct behavior to delete a potential CSE candidate and it might lead to a crash/assert?

[sandreenko]

op2 is always an integer const (1) and CSE doesn't work for them without a special switch -- so it's not a correct behavior to delete a potential CSE candidate and it might lead to a crash/assert?

the switch is on for arm64 but tree can be a CSE candidate without it. I would recommend adding a change to protect against it.