All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ralf Baechle <ralf@linux-mips.org>
To: Dominic Sweetman <dom@mips.com>
Cc: Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de>,
	linux-mips@linux-mips.org, Nigel Stephens <nigel@mips.com>,
	David Ung <davidu@mips.com>
Subject: Re: [PATCH] Improve atomic.h implementation robustness
Date: Wed, 1 Dec 2004 13:33:36 +0100	[thread overview]
Message-ID: <20041201123336.GA5612@linux-mips.org> (raw)
In-Reply-To: <16813.39660.948092.328493@doms-laptop.algor.co.uk>

On Wed, Dec 01, 2004 at 10:20:28AM +0000, Dominic Sweetman wrote:

> > the atomic functions use so far memory references for the inline
> > assembler to access the semaphore. This can lead to additional
> > instructions in the ll/sc loop, because newer compilers don't
> > expand the memory reference any more but leave it to the assembler.
> > 
> > The appended patch...
> 
> I thought it was an explicit aim of the substantial rewrite of the
> MIPS backend for 3.x to get the compiler to generate only "real"
> instructions, not macros which expand to multiple instructions inside
> the assembler.  So it's disappointing if newer compilers got worse.
> 
> Can one of our compiler-knowledgable people follow this up?

Dominik,

this problem here is specific to inline assembler.  The splitlock code for
a reasonable CPU is:

static __inline__ void atomic_add(int i, atomic_t * v)
{
        unsigned long temp;

        __asm__ __volatile__(
        "1:     ll      %0, %1          # atomic_add            \n"
        "       addu    %0, %2                                  \n"
        "       sc      %0, %1                                  \n"
        "       beqz    %0, 1b                                  \n"
        : "=&r" (temp), "=m" (v->counter)
        : "Ir" (i), "m" (v->counter));
}

For the average atomic op generated code is going to look about like:

80100634:       lui     a0,0x802c
80100638:       ll      a0,-24160(a0)
8010063c:       addu    a0,a0,v0
80100640:       lui     at,0x802c
80100644:       addu    at,at,v1
80100648:       sc      a0,-24160(at)
8010064c:       beqz    a0,80100634 <init+0x194>
80100650:       nop

It's significantly worse for 64-bit due to the excessive code sequence
generated for loading a 64-bit address.  One outside CKSEGx that is.

On 32-bit Thiemo's patch would cut that down to something like:

80100630:       lui     t0,0x802c
80100634:       addiu	t0,t0,-24160
80100638:       ll      a0,0(t0)
8010063c:       addu    a0,a0,v0
80100648:       sc      a0,0(to)
8010064c:       beqz    a0,80100638 <init+0x194>
80100650:       nop

On 64-bit the savings would be even more significant.  But what we actually
want would be using the "o" constraint.  Which just at least on the
compilers where I've tried it, didn't produce code any different from "m".
The expected code would be something like:

80100634:       lui     t0,0x802c
80100638:       ll      a0,-24160(t0)
8010063c:       addu    a0,a0,v0
80100648:       sc      a0,-24160(to)
8010064c:       beqz    a0,80100634 <init+0x194>
80100650:       nop

So another instruction less.

  Ralf

  reply	other threads:[~2004-12-01 12:35 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-12-01  7:00 [PATCH] Improve atomic.h implementation robustness Thiemo Seufer
2004-12-01 10:20 ` Dominic Sweetman
2004-12-01 10:20   ` Dominic Sweetman
2004-12-01 12:33   ` Ralf Baechle [this message]
2004-12-01 21:50     ` Maciej W. Rozycki
2004-12-01 23:39       ` Ralf Baechle
2004-12-02  0:01         ` Maciej W. Rozycki
2004-12-07 11:56       ` Richard Sandiford
2004-12-07 12:56         ` Thiemo Seufer
2004-12-07 19:28           ` Richard Sandiford
2004-12-08  0:09             ` Thiemo Seufer
2004-12-01 20:45   ` Thiemo Seufer
2004-12-01 21:53     ` Maciej W. Rozycki
2004-12-01 23:03       ` Thiemo Seufer
2004-12-02  0:17         ` Maciej W. Rozycki
2004-12-02  1:02           ` Thiemo Seufer
2004-12-02  8:01         ` Dominic Sweetman
2004-12-02  8:38           ` Thiemo Seufer
2004-12-02 11:31             ` Stephen P. Becker
2004-12-02 11:54               ` Maciej W. Rozycki
2004-12-02 11:57                 ` Stephen P. Becker

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=20041201123336.GA5612@linux-mips.org \
    --to=ralf@linux-mips.org \
    --cc=davidu@mips.com \
    --cc=dom@mips.com \
    --cc=ica2_ts@csv.ica.uni-stuttgart.de \
    --cc=linux-mips@linux-mips.org \
    --cc=nigel@mips.com \
    /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.