Unnecessary shr emitted for repeated multiplication

[danmoseley]

Noted in discussion here about the recent DateTime perf optimization, that we could not rely on the JIT to safely collapse multiplication. In this example below, there's an extra shr. Same on x86.

sharplab

class Program
{
    uint f(uint u2) {
        return u2 / 2939745 / 4;
    }

    uint g(uint u2) {
        return u2 / 11758980;
    }
}

Program.f(UInt32)
    L0000: mov eax, edx
    L0002: imul rax, 0x5b4fffcb
    L0009: shr rax, 0x34
    L000d: shr eax, 2
    L0010: ret

Program.g(UInt32)
    L0000: mov eax, edx
    L0002: imul rax, 0x5b4fffcb
    L0009: shr rax, 0x36
    L000d: ret

cc @cassioneri

category:cq
theme:basic-cq
skill-level:beginner
cost:small
impact:small

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Noted in discussion here about the recent DateTime perf optimization, that we could not rely on the JIT to safely collapse multiplication. In this example below, there's an extra shr. Same on x86.

sharplab

class Program
{
    uint f(uint u2) {
        return u2 / 2939745 / 4;
    }

    uint g(uint u2) {
        return u2 / 11758980;
    }
}

Program.f(UInt32)
    L0000: mov eax, edx
    L0002: imul rax, 0x5b4fffcb
    L0009: shr rax, 0x34
    L000d: shr eax, 2
    L0010: ret

Program.g(UInt32)
    L0000: mov eax, edx
    L0002: imul rax, 0x5b4fffcb
    L0009: shr rax, 0x36
    L000d: ret

cc @cassioneri

Author:	danmoseley
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[danmoseley]

Can repeated shr (or shl) be safely collapsed as a peephole optimization? Or are there subtleties/sideeffects that cannot be done in that way.

[danmoseley]

Adding the Godbolt link from @cassioneri for gcc/clang: https://godbolt.org/z/T7M8Pqq1e .. MSVC seems to miss the same thing.

[EgorBo]

Looks like a good first issue for jit first-time contributors 🙂

[SkiFoD]

Hey, I would like to try to work with this issue.