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

Synthesizing labels for jump targets #546

Closed
adamsitnik opened this Issue Sep 10, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@adamsitnik
Member

adamsitnik commented Sep 10, 2017

From private conversation with @JosephTremoulet

synthesizing labels for jump targets, emitting them in the instr stream and annotating which one each jump references; it makes the structure of the method apparent in a way that the flat list obscures, it’s always the first transform I do by hand when looking at disasm that hasn’t done it automatically

This should be easy ;)

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Dec 7, 2017

Member

I implemented new exporter, which prints a pretty output.

Sample input:

public static bool LoopGoto_(String strA, String strB)
{
    int length = strA.Length;

    fixed (char* ap = strA) fixed (char* bp = strB)
    {
        char* a = ap;
        char* b = bp;

        while (length != 0)
        {
            int charA = *a;
            int charB = *b;

            if (charA != charB)
                goto ReturnFalse;

            a++;
            b++;
            length--;
        }

        return true;

        ReturnFalse:
        return false;
    }
}

Before (raw output exporter kept to show the source asm code in case my implementation breaks something):

image

After:

image

@JosephTremoulet would you like to give it a try and provide some feedback? The package should be available after 0.10.11.362 build finishes.

Member

adamsitnik commented Dec 7, 2017

I implemented new exporter, which prints a pretty output.

Sample input:

public static bool LoopGoto_(String strA, String strB)
{
    int length = strA.Length;

    fixed (char* ap = strA) fixed (char* bp = strB)
    {
        char* a = ap;
        char* b = bp;

        while (length != 0)
        {
            int charA = *a;
            int charB = *b;

            if (charA != charB)
                goto ReturnFalse;

            a++;
            b++;
            length--;
        }

        return true;

        ReturnFalse:
        return false;
    }
}

Before (raw output exporter kept to show the source asm code in case my implementation breaks something):

image

After:

image

@JosephTremoulet would you like to give it a try and provide some feedback? The package should be available after 0.10.11.362 build finishes.

@JosephTremoulet

This comment has been minimized.

Show comment
Hide comment
@JosephTremoulet

JosephTremoulet Dec 7, 2017

Big improvement, thanks!

A couple stray thoughts:

  1. It would be cool if there were some easier way to find references to a given label other than scanning the listing for them visually. I don't really have ideas for how to do that, though. I've seen dumps that include reference counts next to labels, but not convinced that would be an improvement. An explicit predecessor list might be nice, but to make that clear you'd probably want the labels to identify basic blocks, which would mean inject labels after conditional branches so blocks are single-exit, which in turn would mean you'd have zero-ref labels that are confusing without the ref counts...
    I guess you could do something like label the branches, e.g.

        jne    M01_L04   ; jcc#3
    ...
    M01_L04  ; refs: jcc#3, jcc#7
    

    (the idea being that the jcc#s would be sorted by where the branch is, making them easier to find)
    but I'm probably over-thinking this...

  2. You've suppressed the encoded bytes as well as the addresses. That's probably for the best, though it's not uncommon to make changes whose primary benefit is to use shorter encodings for things. So it would maybe (?) be nice to bring back some indication of the size of each instruction, perhaps in a trailing comment, but I'd guess that would probably just clutter things more than would be worth it...

JosephTremoulet commented Dec 7, 2017

Big improvement, thanks!

A couple stray thoughts:

  1. It would be cool if there were some easier way to find references to a given label other than scanning the listing for them visually. I don't really have ideas for how to do that, though. I've seen dumps that include reference counts next to labels, but not convinced that would be an improvement. An explicit predecessor list might be nice, but to make that clear you'd probably want the labels to identify basic blocks, which would mean inject labels after conditional branches so blocks are single-exit, which in turn would mean you'd have zero-ref labels that are confusing without the ref counts...
    I guess you could do something like label the branches, e.g.

        jne    M01_L04   ; jcc#3
    ...
    M01_L04  ; refs: jcc#3, jcc#7
    

    (the idea being that the jcc#s would be sorted by where the branch is, making them easier to find)
    but I'm probably over-thinking this...

  2. You've suppressed the encoded bytes as well as the addresses. That's probably for the best, though it's not uncommon to make changes whose primary benefit is to use shorter encodings for things. So it would maybe (?) be nice to bring back some indication of the size of each instruction, perhaps in a trailing comment, but I'd guess that would probably just clutter things more than would be worth it...

@redknightlois

This comment has been minimized.

Show comment
Hide comment
@redknightlois

redknightlois Dec 7, 2017

Contributor

Agree on 2. At least being able to turn them on somehow.

Contributor

redknightlois commented Dec 7, 2017

Agree on 2. At least being able to turn them on somehow.

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Dec 7, 2017

Member

@JosephTremoulet @redknightlois thanks for feedback!

speaking of 2. Would a tooltip with raw value would be enough? So anytime user mouse over the pretty text, the tool tip appears?

image

I have no good idea for the search, except of pressing Ctrl+F in browser and going with F3 to next occurence

Member

adamsitnik commented Dec 7, 2017

@JosephTremoulet @redknightlois thanks for feedback!

speaking of 2. Would a tooltip with raw value would be enough? So anytime user mouse over the pretty text, the tool tip appears?

image

I have no good idea for the search, except of pressing Ctrl+F in browser and going with F3 to next occurence

@ilabutin

This comment has been minimized.

Show comment
Hide comment
@ilabutin

ilabutin Dec 7, 2017

I have no good idea for the search, except of pressing Ctrl+F in browser and going with F3 to next occurence

@adamsitnik You produce HTML, so what about including a small bit of JS into it which will handle mouse hovering the label and highlight all places where it is used?
I can offer some help if needed.

This will not solve the case when label is used outside of visible area, though. But we may think of handling clicks instead of hover. So that when you click on the label all usages are highlighted "permanently" until you click another label.

ilabutin commented Dec 7, 2017

I have no good idea for the search, except of pressing Ctrl+F in browser and going with F3 to next occurence

@adamsitnik You produce HTML, so what about including a small bit of JS into it which will handle mouse hovering the label and highlight all places where it is used?
I can offer some help if needed.

This will not solve the case when label is used outside of visible area, though. But we may think of handling clicks instead of hover. So that when you click on the label all usages are highlighted "permanently" until you click another label.

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Dec 9, 2017

Member

@ilabutin thanks for a good hint!

What I implemented:

  1. When user hovers over a label, the mouse cursor changes to pointer and label get's highlighted
  2. When user clicks a label, all usages gets highlighted
  3. When user presses F3, we jump to next usage of given label

Small demo:

labelssample

Member

adamsitnik commented Dec 9, 2017

@ilabutin thanks for a good hint!

What I implemented:

  1. When user hovers over a label, the mouse cursor changes to pointer and label get's highlighted
  2. When user clicks a label, all usages gets highlighted
  3. When user presses F3, we jump to next usage of given label

Small demo:

labelssample

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

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

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