New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mono Support for DisassemblyDiagnoser #541

Closed
AndreyAkinshin opened this Issue Sep 7, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@AndreyAkinshin
Member

AndreyAkinshin commented Sep 7, 2017

Consider the following benchmarks:

 [DisassemblyDiagnoser(printAsm: true, printSource: true)] // !!! use the new diagnoser!!
    [Config(typeof(Config))]
    public class Simple
    {
        private class Config : ManualConfig
        {
            public Config()
            {
                Add(Job.Dry.With(Runtime.Mono));
            }
        }
        int[] field = Enumerable.Range(0, 100).ToArray();

        [Benchmark]
        public int SumLocal()
        {
            var local = field; // we use local variable that points to the field

            int sum = 0;
            for (int i = 0; i < local.Length; i++)
                sum += local[i];

            return sum;
        }

        [Benchmark]
        public int SumField()
        {
            int sum = 0;
            for (int i = 0; i < field.Length; i++)
                sum += field[i];

            return sum;
        }
    }

Here are my disasm results on Linux and Windows:
simple-linux
simple-windows
It doesn't look like a correct results.

@adamsitnik

This comment has been minimized.

Member

adamsitnik commented Sep 10, 2017

It's a bug, however I am afraid that I will not have any free time soon to fix it (I have 3 conferences next week). I am going to mark it as up-for-grabs.

@adamsitnik

This comment has been minimized.

Member

adamsitnik commented Sep 10, 2017

How to fix the problem:

  1. Compile the code provided by @AndreyAkinshin
  2. Run mono -v -v -v -v --compile $namespace.$typeName:$methodName $pathToExe
  3. Take it's output, add it as a new test to this class with tests
  4. Modify our parser to get the tests passing

@adamsitnik adamsitnik added the bug label Sep 10, 2017

@AndreyAkinshin AndreyAkinshin self-assigned this Sep 11, 2017

@AndreyAkinshin

This comment has been minimized.

Member

AndreyAkinshin commented Sep 11, 2017

I will do it, thanks for the guide.

@AndreyAkinshin

This comment has been minimized.

Member

AndreyAkinshin commented Sep 11, 2017

@adamsitnik, current implementations of MonoDisassembler and MonoDisassemblyOutputParserTests look strange to me. Correct assembly code for BenchmarkDotNet.Samples.CPU.Cpu_Atomics:NoLock on my Linux+Mono5.2 is:

0000000000000000 <chmarkDotNet_Samples_CPU_Cpu_Atomics_NoLock>:
<BB>:2
   0:	48 83 ec 08          	sub    $0x8,%rsp
   4:	4c 89 3c 24          	mov    %r15,(%rsp)
   8:	4c 8b ff             	mov    %rdi,%r15
   b:	49 63 47 18          	movslq 0x18(%r15),%rax
   f:	ff c0                	inc    %eax
  11:	41 89 47 18          	mov    %eax,0x18(%r15)
  15:	ff c0                	inc    %eax
  17:	41 89 47 18          	mov    %eax,0x18(%r15)
  1b:	ff c0                	inc    %eax
  1d:	41 89 47 18          	mov    %eax,0x18(%r15)
  21:	ff c0                	inc    %eax
  23:	41 89 47 18          	mov    %eax,0x18(%r15)
<BB>:1
  27:	4c 8b 3c 24          	mov    (%rsp),%r15
  2b:	48 83 c4 08          	add    $0x8,%rsp
  2f:	c3                   	retq  

However, we expect only some intermediate lines:

new Diagnosers.Code { TextRepresentation = "loadi4_membase %eax <- [%edi + 0xc]" },

new Diagnosers.Code { TextRepresentation = "int_add_imm %eax <- %eax [1] clobbers: 1" },
new Diagnosers.Code { TextRepresentation = "storei4_membase_reg [%edi + 0xc] <- %eax" },
new Diagnosers.Code { TextRepresentation = "move %eax <- %eax" },

new Diagnosers.Code { TextRepresentation = "int_add_imm %eax <- %eax [1] clobbers: 1" },
new Diagnosers.Code { TextRepresentation = "storei4_membase_reg [%edi + 0xc] <- %eax" },
new Diagnosers.Code { TextRepresentation = "move %eax <- %eax" },

new Diagnosers.Code { TextRepresentation = "int_add_imm %eax <- %eax [1] clobbers: 1" },
new Diagnosers.Code { TextRepresentation = "storei4_membase_reg [%edi + 0xc] <- %eax" },
new Diagnosers.Code { TextRepresentation = "move %eax <- %eax" },

new Diagnosers.Code { TextRepresentation = "int_add_imm %eax <- %eax [1] clobbers: 1" },
new Diagnosers.Code { TextRepresentation = "storei4_membase_reg [%edi + 0xc] <- %eax" },

Am I right that we parse only this stuff for now (without honest disasm)? Is it ok if I rewrite MonoDisassembler complitely? (I also would like to use the MONO_VERBOSE_METHOD approach instead of simple -v -v -v -v which produce too much output lines.)

@adamsitnik

This comment has been minimized.

Member

adamsitnik commented Sep 11, 2017

@AndreyAkinshin I started from trying MONO_VERBOSE_METHOD as suggested by Miguel. But on my machine (Windows 8 & 10, various Mono builds) I was always getting only the "intermidiate" diasm. This is why I implemented it that way.

I would be very happy if you can re-implement it in better way. But please make sure that it works on Windows in the same way.

@AndreyAkinshin

This comment has been minimized.

Member

AndreyAkinshin commented Sep 11, 2017

Ok, will do it.

@AndreyAkinshin

This comment has been minimized.

Member

AndreyAkinshin commented Sep 12, 2017

But please make sure that it works on Windows in the same way.

Unfortunately, it seems that it's impossible to implement a completely portable solution. Mono just uses external objdump and as tools. For example, on Mono 5.2+Windows, it just calls x86_64-w64-mingw32-objdump.exe (see https://github.com/mono/mono/blob/mono-5.2.0.215/mono/mini/helpers.c#L222). If you don't have it, you will not get the assembly code. We can detect such environments and provide an instruction about how to make it work.
Right now we print some kind of debug JIT data which can be interesting, but it's really hard to work with it (e.g. compare it with assembly code in .NET Framework or .NET Core). We can keep it as a fallback way on Windows (in case if we don't have objdump or as), but I'm not sure that it will be a useful feature without real asm instructions.

@adamsitnik

This comment has been minimized.

Member

adamsitnik commented Sep 12, 2017

@AndreyAkinshin great explanation!

We can keep it as a fallback way on Windows (in case if we don't have objdump or as),

I agree, this is the best solution.

@AndreyAkinshin AndreyAkinshin changed the title from Problems with Disassembler on Mono to Mono Support for DisassemblyDiagnoser Nov 16, 2017

morgan-kn added a commit to morgan-kn/BenchmarkDotNet that referenced this issue Jan 30, 2018

@AndreyAkinshin AndreyAkinshin added this to the v0.10.13 milestone Jan 31, 2018

AndreyAkinshin added a commit that referenced this issue Jan 31, 2018

Merge pull request #637 from morgan-kn/MonoSupportForDisassemblyDiagn…
…oser

Mono Support for DisassemblyDiagnoser #541
@AndreyAkinshin

This comment has been minimized.

Member

AndreyAkinshin commented Jan 31, 2018

Fixed by #637, thanks @morgan-kn!

GeorgePlotnikov added a commit to GeorgePlotnikov/BenchmarkDotNet that referenced this issue Feb 25, 2018

alinasmirnova added a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment