RyuJIT: dangling instructions after converting modulus to bitwise AND

[omariom]

Looks like after optimization a few instructions left dangling.

[MethodImpl(MethodImplOptions.NoInlining)]
 public static uint Test(uint v)
 {
       return v % 8;
 }

[MethodImpl(MethodImplOptions.NoInlining)]
public static uint Test2(uint v)
{
    return v & 7;
}

;Test
G_M65206_IG02:
       8BC1                 mov      eax, ecx
       33D2                 xor      rdx, rdx
       8BC1                 mov      eax, ecx
       83E007               and      eax, 7
G_M65206_IG03:
       C3                   ret 

;Test2
G_M65206_IG02:
       8BC1                 mov      eax, ecx
       83E007               and      eax, 7
G_M65206_IG03:
       C3                   ret   

[mikedn]

That's due to the codegen for GT_UMOD which prepares to emit a DIV by zero extending in edx:eax but later changes its mind and emits the AND version: codegenxarch.cpp:1226. Probably this can be fixed by doing the power of 2 check before attempting to move the value to edx:eax, provided that genCodeForPow2Div is changed to not assume that edx contains a correct value in signed cases.

That said, it is somewhat peculiar that this optimization is done during codegen. For codegen to be able to use edx:eax the register allocator must have probably already reserved those registers for a GT_UMOD. That means that the registers won't be available for other uses that span the GT_UMOD even if AND is used instead of DIV. This optimization should have probably been done before register allocation and codegen.

[ghost]

@omariom, aside; can you please tell me how are you generating assembly code from coreclr? I built the coreclr solution and compiled my C# code and ran the compiled application with corerun.exe (all with command line interface), but couldn't figure out how to emit the asm code.. Do i need to install some other tool or is there a command-line way of doing it with the tools generated by coreclr solution?

[omariom]

@jasonwilliams200OK

Set COMPLUS_JitDisasm environment variable to name of the method you want to disasm.

[ghost]

Thanks @omariom, I wanted to disasm Main method so I did Set COMPLUS_JitDisasm=Main followed by csc.exe app.cs and then corerun app.exe. Where can I find the assembly?

[mikedn]

@jasonwilliams200OK The disassembly should be displayed to the console. Make sure you run a debug build of coreclr, release builds do not have this feature.

[ghost]

Thanks @mikedn ! It worked :)

For future reference, assuming the method to disassemble is MyMethod:

::: Build CoreCLR (Debug) :::
git clone https://github.com/dotnet/coreclr
cd coreclr
\build.cmd  :: takes quite some time!
::: /Build CoreCLR :::

SET COMPLUS_JitDisasm=MyMethod    :: this is if you are in cmd
:: for powershell set env var as (notice the quotes):
:: $env:COMPLUS_JitDisasm = 'MyMethod'

cd bin\Product\Windows_NT.x64.Debug

notepad someApp.cs
:: write hello world console code in there and add a method called MyMethod
:: save and close notepad

.\csc someApp.cs
.\corerun someApp.exe

:: will spit out the disassembly for MyMethod in console, which can be piped to the file:
:: .\corerun someApp.exe > c:\temp\MyMethod.disasm.txt

More info at ryujit-overview.md#dumps-and-other-tools and clr-configuration-knobs.md.

@agocke, you once asked similar question on gitter. Is this what you were looking for?

[mikedn]

For codegen to be able to use edx:eax the register allocator must have probably already reserved those registers for a GT_UMOD .

Indeed, fixing the codegen doesn't help in other cases:

using System.Runtime.CompilerServices;
class Program {
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static uint Test(uint x, uint y) {
        return y + (x % 4);
    }

    static int Main(string[] args) {
        return (int)Test((uint)args.Length, (uint)args.Length);
    }
}

produces

G_M31886_IG01:
       89542410             mov      dword ptr [rsp+10H], edx

G_M31886_IG02:
       8BC1                 mov      eax, ecx
       83E003               and      eax, 3
       8B542410             mov      edx, dword ptr [rsp+10H]
       03C2                 add      eax, edx

G_M31886_IG03:
       C3                   ret

instead of

G_M31886_IG01:

G_M31886_IG02:
       8BC1                 mov      eax, ecx
       83E003               and      eax, 3
       03C2                 add      eax, edx

G_M31886_IG03:
       C3                   ret

[RussKeldorph]

Reopen since dotnet/coreclr#1241 was backed out.