On Wed, 25 Jul 2007, Linus Torvalds wrote:
>
> Hmm. I really think you should take this up with the gcc people. That
> looks like a gcc bug - because there really is nothing that guarantees
> that the asm doesn't change the array that "x" points to, and the asm
> clearly talks about clobberin
On Wed, 25 Jul 2007, Trent Piepho wrote:
>
> Specifically, check test6_memasm.s. The C code looks like this:
>
> extern int a; /* keep asm from being elided for having no used output */
> static inline void bar(void) { asm("call bar" : "=m"(a) : : "memory"); }
> /* float x can't alias asm's ou
On Tue, 24 Jul 2007, Linus Torvalds wrote:
> On Tue, 24 Jul 2007, Trent Piepho wrote:
> >
> > Speaking of that, why are all the asm functions in arch/i386/lib/string.c
> > defined as having a memory clobber, even those which don't modify memory
> > like strcmp, strchr, strlen and so on?
>
> That's
Benjamin Herrenschmidt wrote:
On Tue, 2007-07-24 at 17:55 -0400, Trond Myklebust wrote:
If you want to use bitops as spinlocks you should rather be using
. That also does the right thing w.r.t.
pre-emption and sparse locking annotations.
Heh, I didn't know about those... A bit annoying that
On Tue, 2007-07-24 at 17:55 -0400, Trond Myklebust wrote:
>
> If you want to use bitops as spinlocks you should rather be using
> . That also does the right thing w.r.t.
> pre-emption and sparse locking annotations.
Heh, I didn't know about those... A bit annoying that I can't override
them in th
On Wed, 2007-07-25 at 07:37 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2007-07-24 at 11:13 -0700, Linus Torvalds wrote:
> >
> > IOW, if you do a spinlock with the bitops, the locking side should be
> > able
> > to use a "test_and_set_bit()" on its own, but the unlocking side
> > should be
> >
On Tue, 24 Jul 2007, Jeremy Fitzhardinge wrote:
> >
> > But gcc docs also talk about the other things volatile means, including
> > "not significantly moved".
>
> Actually, it doesn't. In fact it goes out of its way to say that "asm
> volatile" statements can be moved quite a bit, with respect
On Tue, 2007-07-24 at 11:13 -0700, Linus Torvalds wrote:
>
> IOW, if you do a spinlock with the bitops, the locking side should be
> able
> to use a "test_and_set_bit()" on its own, but the unlocking side
> should be
>
> smp_mb__before_clear_bit();
> clear_bit();
>
> because the
On Tue, 2007-07-24 at 10:24 -0700, Linus Torvalds wrote:
>
> On Tue, 24 Jul 2007, Benjamin Herrenschmidt wrote:
> >
> > In fact, it's more than that... the bitops that return a value are often
> > used to have hand-made spinlock semantics. I'm sure we would get funky
> > bugs if loads or stores l
Linus Torvalds wrote:
> Sure, that's *one* thing that "volatile" guarantees (it guarantees that
> gcc won't optimize away things where the end result isn't actually visibly
> used).
>
> But gcc docs also talk about the other things volatile means, including
> "not significantly moved".
>
Act
On Tue, 24 Jul 2007, Andi Kleen wrote:
>
> Linus Torvalds <[EMAIL PROTECTED]> writes:
> >
> > (Yes, the "asm volatile" may do so too, but it's very unclear what the
> > "volatile" on the asm actually does, so ..)
>
> Without the volatile they get completely optimized away :/
> [tried that, cos
Linus Torvalds <[EMAIL PROTECTED]> writes:
>
> (Yes, the "asm volatile" may do so too, but it's very unclear what the
> "volatile" on the asm actually does, so ..)
Without the volatile they get completely optimized away :/
[tried that, cost a lot of debugging time -- empty string functions
cause
On Tue, 24 Jul 2007, Trent Piepho wrote:
>
> Speaking of that, why are all the asm functions in arch/i386/lib/string.c
> defined as having a memory clobber, even those which don't modify memory
> like strcmp, strchr, strlen and so on?
That's because the memory clobber will serialize the inline
On Tue, 2007-07-24 at 11:13 -0700, Linus Torvalds wrote:
>
> On Tue, 24 Jul 2007, Trond Myklebust wrote:
> >
> > That's not what the Documentation/memory-barriers.txt states:
>
> Hmm.. You're right. We only actually need it for the unconditional bitops,
> like the *unlock* side.
>
> IOW, if yo
On Tue, 24 Jul 2007, Trond Myklebust wrote:
>
> That's not what the Documentation/memory-barriers.txt states:
Hmm.. You're right. We only actually need it for the unconditional bitops,
like the *unlock* side.
IOW, if you do a spinlock with the bitops, the locking side should be able
to use a
On Tue, 2007-07-24 at 10:24 -0700, Linus Torvalds wrote:
>
> On Tue, 24 Jul 2007, Benjamin Herrenschmidt wrote:
> >
> > In fact, it's more than that... the bitops that return a value are often
> > used to have hand-made spinlock semantics. I'm sure we would get funky
> > bugs if loads or stores l
On Tue, 24 Jul 2007, Benjamin Herrenschmidt wrote:
>
> In fact, it's more than that... the bitops that return a value are often
> used to have hand-made spinlock semantics. I'm sure we would get funky
> bugs if loads or stores leaked out of the locked region. I think a full
> "memory" clobber sh
Hi David,
On Tue, 24 Jul 2007, David Howells wrote:
> Satyam Sharma <[EMAIL PROTECTED]> wrote:
>
> > OTOH, as per Linus' review it seems we can drop the "memory" clobber
> > and specify the output operand for the extended asm as "+m". But I
> > must admit I didn't quite understand that at all.
>
On Mon, 2007-07-23 at 10:55 -0700, Linus Torvalds wrote:
> > The goal is to let gcc generate good, beautiful, optimized code.
>
> No. The point is to let gcc generate *correct* code.
>
> It's either "=m" together with "memory", or it's "+m".
In fact, it's more than that... the bitops that return
Satyam Sharma <[EMAIL PROTECTED]> wrote:
> OTOH, as per Linus' review it seems we can drop the "memory" clobber
> and specify the output operand for the extended asm as "+m". But I
> must admit I didn't quite understand that at all.
As I understand it, the "+m" indicates to the compiler a restric
Satyam Sharma wrote:
On Tue, 24 Jul 2007, Nick Piggin wrote:
[...]
__test_and_change_bit is one that you could remove the memory clobber
from.
Yes, for the atomic versions we don't care if we're asking gcc to
generate trashy code (even though I'd have wanted to only disallow
problematic opt
On Tue, 24 Jul 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
> > But for the non-atomic variants, it does make sense to remove the
> > memory clobber (and the unneeded __asm__ __volatile__ that another
> > patch did -- for the non-atomic variants, again).
>
> No. It has nothing to do with atomici
On Tue, 24 Jul 2007, Nick Piggin wrote:
> > > [...]
> > >
> > > __test_and_change_bit is one that you could remove the memory clobber
> > > from.
> >
> > Yes, for the atomic versions we don't care if we're asking gcc to
> > generate trashy code (even though I'd have wanted to only disallow
> > p
Satyam Sharma wrote:
On Tue, 24 Jul 2007, Nick Piggin wrote:
Satyam Sharma wrote:
From: Satyam Sharma <[EMAIL PROTECTED]>
[6/8] i386: bitops: Don't mark memory as clobbered unnecessarily
The goal is to let gcc generate good, beautiful, optimized code.
But test_and_set_bit, test_and_clear_
On Tue, 24 Jul 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
> > From: Satyam Sharma <[EMAIL PROTECTED]>
> >
> > [6/8] i386: bitops: Don't mark memory as clobbered unnecessarily
> >
> > The goal is to let gcc generate good, beautiful, optimized code.
> >
> > But test_and_set_bit, test_and_cle
Satyam Sharma wrote:
From: Satyam Sharma <[EMAIL PROTECTED]>
[6/8] i386: bitops: Don't mark memory as clobbered unnecessarily
The goal is to let gcc generate good, beautiful, optimized code.
But test_and_set_bit, test_and_clear_bit, __test_and_change_bit,
and test_and_change_bit unnecessarily
On Mon, 23 Jul 2007, Satyam Sharma wrote:
>
> The goal is to let gcc generate good, beautiful, optimized code.
No. The point is to let gcc generate *correct* code.
It's either "=m" together with "memory", or it's "+m".
You just introduced a bug.
Linus
-
To unsubscribe from th
Satyam Sharma wrote:
> Exactly, but the actual _synchronization_ in all users of the bitops API
> should (should, at least, otherwise the bugs lie in the callers) depend
> upon the _bit-string_ whose address is passed to us. That could be some
> flags/lock/etc in some caller, whatever, but all the
Hi,
On Mon, 23 Jul 2007, Andi Kleen wrote:
>
> > Yes, but _that_ address (of the bit-string) is protected already -- by the
> > implicit memory barrier due to the LOCK prefix.
>
> Compiler barrier != CPU barrier.
Exactly, but the actual _synchronization_ in all users of the bitops API
should (
> Yes, but _that_ address (of the bit-string) is protected already -- by the
> implicit memory barrier due to the LOCK prefix.
Compiler barrier != CPU barrier.
The memory clobber is a compiler barrier that prevents its global optimizer
from moving memory references. The CPU memory ordering guara
Hi Andi,
On Mon, 23 Jul 2007, Andi Kleen wrote:
> On Monday 23 July 2007 18:05:58 Satyam Sharma wrote:
> > From: Satyam Sharma <[EMAIL PROTECTED]>
> >
> > [6/8] i386: bitops: Don't mark memory as clobbered unnecessarily
> >
> > The goal is to let gcc generate good, beautiful, optimized code.
>
On Monday 23 July 2007 18:05:58 Satyam Sharma wrote:
> From: Satyam Sharma <[EMAIL PROTECTED]>
>
> [6/8] i386: bitops: Don't mark memory as clobbered unnecessarily
>
> The goal is to let gcc generate good, beautiful, optimized code.
The first goal is correct code.
The reason for the memory bar
From: Satyam Sharma <[EMAIL PROTECTED]>
[6/8] i386: bitops: Don't mark memory as clobbered unnecessarily
The goal is to let gcc generate good, beautiful, optimized code.
But test_and_set_bit, test_and_clear_bit, __test_and_change_bit,
and test_and_change_bit unnecessarily mark all of memory as c
33 matches
Mail list logo
