dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.19k stars 4.72k forks source link

[JIT] X64 - Use containment to use memory operands for `blsi`, `blsmsk`, and `blsr` #81515

Open TIHan opened 1 year ago

TIHan commented 1 year ago
public class C 
{
    public uint _bitfield;

    uint blsi()
    {
        return _bitfield & (0u - _bitfield);
    }

    uint blsmsk()
    {
        return _bitfield ^ (_bitfield - 1);
    }

    uint blsr()
    {
        return _bitfield & (_bitfield - 1);
    }
}

Currently emits:

C.blsi()
    mov eax, [rcx+8]
    blsi eax, eax
    ret

C.blsmsk()
    mov eax, [rcx+8]
    blsmsk eax, eax
    ret

C.blsr()
    mov eax, [rcx+8]
    blsr eax, eax
    ret

Instead, it should emit:

blsi():
        blsi    eax, dword ptr [rcx+8]
        ret
blsmsk():     
        blsmsk  eax, dword ptr [rcx+8]
        ret
blsr():
        blsr    eax, dword ptr [rcx+8]
        ret
ghost commented 1 year ago

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

Issue Details
```csharp public class C { public uint _bitfield; uint blsi() { return _bitfield & (0u - _bitfield); } uint blsmsk() { return _bitfield ^ (_bitfield - 1); } uint blsr() { return _bitfield & (_bitfield - 1); } } ``` Currently emits: ``` C.blsi() mov eax, [rcx+8] blsi eax, eax ret C.blsmsk() mov eax, [rcx+8] blsmsk eax, eax ret C.blsr() mov eax, [rcx+8] blsr eax, eax ret ``` Instead, it should emit: ``` blsi(): blsi eax, dword ptr [rcx+8] ret blsmsk(unsigned int): blsmsk eax, dword ptr [rcx+8] ret blsr(unsigned int): blsr eax, dword ptr [rcx+8] ret ```
Author: TIHan
Assignees: TIHan
Labels: `tenet-performance`, `area-CodeGen-coreclr`
Milestone: 8.0.0
Wraith2 commented 1 year ago

Any guidance on what changes are needed to make that happen?

TIHan commented 1 year ago

Any guidance on what changes are needed to make that happen?

Appreciate your interest in this issue, but I'm going to be doing this work as a larger set of work items that I have.

Wraith2 commented 1 year ago

Ok, that's a bit of a relief. I don't really understand containment.