Messages in this thread | | | Date | Tue, 24 Jul 2007 18:39:15 +1000 | From | Nick Piggin <> | Subject | Re: [PATCH 6/8] i386: bitops: Don't mark memory as clobbered unnecessarily |
| |
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 optimizations -- ones involving the passed bit-string >>>address -- there, and allow other memory references to be optimized >>>as and how the compiler feels like it) because the atomic variants >>>are slow anyway and we probably want to be extra-safe there. >>> >>>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 atomicity and all to do with ordering. > > > The memory clobber, or the volatile asm? There's more than one variable > here ... but still, I don't think either affects _ordering_ in any > _direct_ way.
The clobber which you remove with this patch.
>>For example test_bit, clear_bit, set_bit, etc are all atomic but none >>place any restrictions on ordering. > > > In that case we need to update comments in include/asm-i386/bitops.h
Hmm... yeah it looks like they could be reordered. I think?
>>__test_and_change_bit has no restriction on ordering, so as long as >>the correct operands are clobbered, a "memory" clobber to enforce a >>compiler barrier is not needed. > > > But why even for the other operations? Consider (current code of) > test_and_set_bit(): > > static inline int test_and_set_bit(int nr, volatile unsigned long * addr) > { > int oldbit; > __asm__ __volatile__( LOCK_PREFIX > "btsl %2,%1\n\tsbbl %0,%0" > :"=r" (oldbit),"+m" (ADDR) > :"Ir" (nr) : "memory"); > > return oldbit; > } > > The only memory reference in there is to the passed address, it will > be modified, yes, but that's been made obvious to gcc in the asm > already. So why are we marking all of memory as clobbered, is the > question. (I just read Jeremy's latest reply, but I don't see how > or why the memory clobber helps that case either -- does a memory > clobber affect how gcc would order / reorder code?)
Of course, because then the compiler can't assume anything about the contents of memory after the operation.
#define barrier() __asm__ __volatile__("": : :"memory")
A memory clobber is equivalent to a compiler barrier.
-- SUSE Labs, Novell Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |