All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulrich Drepper <drepper@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	davidel@xmailserver.org, mtk.manpages@gmail.com,
	torvalds@linux-foundation.org
Subject: Re: [PATCH 01/18] flag parameters: helper function
Date: Mon, 05 May 2008 19:39:34 -0700	[thread overview]
Message-ID: <481FC4E6.5060206@redhat.com> (raw)
In-Reply-To: <20080505185103.b9299541.akpm@linux-foundation.org>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Morton wrote:
> hm, that looks expensive.  The compiler will need to generate a deref of m
> and rf multiple times around the loop.  Copying them into locals does
> improve that a lot.

There really is no problem.  The value is in L1d when it is reused.
This is the generated code (%rdi is m):

   f:   85 17                   test   %edx,(%rdi)
  11:   74 0b                   je     1e <flags_remap+0x1e>
  13:   8b 47 04                mov    0x4(%rdi),%eax
  16:   09 01                   or     %eax,(%rcx)
  18:   8b 07                   mov    (%rdi),%eax
  1a:   f7 d0                   not    %eax
  1c:   21 c2                   and    %eax,%edx

At address 18 the load will be satisfied from L1d.  If you would want to
cache the value at address f you'd have to create one more instruction.

This really is the best code sequence.  The compiler could have chosen
to move the value into a register because the array is const.  But it
didn't.


> Also: sorry, but ugh-at-the-naming.  We don't *gain* anything from having
> idenitifers called f, of, m, n and rf.  And we lose quite a lot in
> readability and understandability.  It would be much nicer to invest a
> little bit more typing-time here, IMO.

That's Davide's code and I didn't change it because it doesn't really
matter.  This is a trivial function which doesn't need more than 10
seconds to be understood.  If you insist I'll rename the variables and
elements but I consider this just busy work.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQFIH8Tl2ijCOnn/RHQRAm+PAKDEArdgKWXLAMzzfYQ3Q8XzbJdWmgCfavq+
+THu/JEISm+IrX7oyhITYVU=
=F2Co
-----END PGP SIGNATURE-----

  parent reply	other threads:[~2008-05-06  2:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-05  3:42 [PATCH 01/18] flag parameters: helper function Ulrich Drepper
2008-05-05  4:45 ` Davide Libenzi
2008-05-05  5:02   ` Ulrich Drepper
2008-05-06  1:51 ` Andrew Morton
2008-05-06  2:09   ` Davide Libenzi
2008-05-06  2:39   ` Ulrich Drepper [this message]
2008-05-06  2:56     ` Andrew Morton

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=481FC4E6.5060206@redhat.com \
    --to=drepper@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=davidel@xmailserver.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=torvalds@linux-foundation.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.