All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: David Laight <David.Laight@aculab.com>
Cc: Ian Rogers <irogers@google.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"linux-perf-users@vger.kernel.org"
	<linux-perf-users@vger.kernel.org>
Subject: Re: [PATCH] perf annotate: Add more x86 mov instruction cases
Date: Wed, 13 Sep 2023 14:14:42 -0700	[thread overview]
Message-ID: <CAM9d7ciB-Rmsi4eTOU7n=mcEP2-JjDycL6f_8cKnKGKtqVT3UQ@mail.gmail.com> (raw)
In-Reply-To: <ad8a8d46151142a883d2d259c884acc0@AcuMS.aculab.com>

On Mon, Sep 11, 2023 at 1:12 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Namhyung Kim
> > Sent: 09 September 2023 00:56
> >
> > Hi Ian,
> >
> > On Thu, Sep 7, 2023 at 11:24 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Thu, Sep 7, 2023 at 10:22 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > >
> > > > Instructions with sign- and zero- extention like movsbl and movzwq were
> > > > not handled properly.  As it can check different size suffix (-b, -w, -l
> > > > or -q) we can omit that and add the common parts even though some
> > > > combinations are not possible.
> > > >
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > ---
> > > >  tools/perf/arch/x86/annotate/instructions.c | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/tools/perf/arch/x86/annotate/instructions.c
> > b/tools/perf/arch/x86/annotate/instructions.c
> > > > index 5f4ac4fc7fcf..5cdf457f5cbe 100644
> > > > --- a/tools/perf/arch/x86/annotate/instructions.c
> > > > +++ b/tools/perf/arch/x86/annotate/instructions.c
> > > > @@ -74,12 +74,15 @@ static struct ins x86__instructions[] = {
> > > >         { .name = "movdqa",     .ops = &mov_ops,  },
> > > >         { .name = "movdqu",     .ops = &mov_ops,  },
> > > >         { .name = "movsd",      .ops = &mov_ops,  },
> > > > -       { .name = "movslq",     .ops = &mov_ops,  },
> > > >         { .name = "movss",      .ops = &mov_ops,  },
> > > > +       { .name = "movsb",      .ops = &mov_ops,  },
> > > > +       { .name = "movsw",      .ops = &mov_ops,  },
> > > > +       { .name = "movsl",      .ops = &mov_ops,  },
> > >
> > > In Intel's manual some of these names are "Move Data From String to
> > > String" operations, movsb and movsw in particular. These instructions
> > > can be used to make simple memcpy loops. Could it be the past omission
> > > was deliberate due to the different way the addressing works in the
> > > instructions?
> >
> > I don't know but in terms of instruction parsing, they are the same
> > "MOVE" with two operands.  I'm not aware of anything in perf with
> > the operands of these instructions.  So I guess it'd be fine to add
> > these instructions even if they have different underlying behaviors.
>
> I'm pretty sure that 'rep movs[bwlq]' (aka while (cx--) *di++ = *si++)
> is likely to be missing the memory argument parameters.
> There is also 'fun and games' with one variant - iirc 'rep movsd'
> what has been used for 64bit, but got hijacked by one of the SIMD sets.

It seems perf annotate don't process the rep prefix yet.
So I think there should be no functional change now.

Thanks,
Namhyung

  reply	other threads:[~2023-09-13 21:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-08  5:22 [PATCH] perf annotate: Add more x86 mov instruction cases Namhyung Kim
2023-09-08  6:23 ` Ian Rogers
2023-09-08 23:56   ` Namhyung Kim
2023-09-11  8:11     ` David Laight
2023-09-13 21:14       ` Namhyung Kim [this message]
2023-09-13 23:24         ` Ian Rogers
2023-09-17  5:28           ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAM9d7ciB-Rmsi4eTOU7n=mcEP2-JjDycL6f_8cKnKGKtqVT3UQ@mail.gmail.com' \
    --to=namhyung@kernel.org \
    --cc=David.Laight@aculab.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.