Prev: [RFC] Yet another (third) dt3155 driver PCI/video4linux compliant
Next: [PATCH] intel-iommu: don't call domain_exit if can not attach with iommu
From: Jamie Lokier on 23 Apr 2010 23:10 Jamie Lokier wrote: > Nicolas Pitre wrote: > > On Thu, 25 Mar 2010, Jamie Lokier wrote: > > > Any one of these should fix it: > > > > > > - Make io_base a pointer-to-volatile-u64 or cast it in the asm, and > > > make sure to dereference it and use an "m" constraint (or > > > tighter, such as "Q", if ldrd needs it). It must be u64, not > > > pointer-to-void, to tell GCC the size. That tells GCC which memory > > > the asm accesses, and the volatile dereference should tell GCC > > > not to reorder them in principle (but the GCC manual doesn't > > > make a specific promise about this for asms). > > > > The LDRD has special range constraints on its addressing mode which is > > not expressable with any of the available gcc memory constraints. > > 'Q' > A memory reference where the exact address is in a single > register (''m'' is preferable for 'asm' statements) > > If 'r' is good enough for io_base, 'Q' should be good enough for *io_base. And if that doesn't work, it's possible to pass both the pointer, and the dereference, as separate input operands, and only use the pointer in the asm template. GCC will still treat the dereference input as a dependency. -- Jamie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Nicolas Pitre on 24 Apr 2010 09:10
On Sat, 24 Apr 2010, Jamie Lokier wrote: > Nicolas Pitre wrote: > > On Thu, 25 Mar 2010, Jamie Lokier wrote: > > > > asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base)); > > > > buf64[i++] = x; > > > > > > The "register...asm" looks fine, but it occurs to me the constraints > > > are too weak (and they were before), so GCC could optimise that to the > > > wrong behaviour. > > > > > > The "volatile" prevents GCC deleting the asm if it's output isn't > > > used, but it doesn't stop GCC from reordering the asms, for example if > > > it decides to unroll the loop. It probably won't reorder in that > > > case, but it could. The result would be out of order values stored > > > into buf[]. It could even move the ldrd earlier than the prior byte > > > accesses, or after the later byte accesses. > > > > I don't see how that could happen. The store into buf[] puts a > > dependency on the output constraint of the inline asm statement. And by > > vertue of being volatile, gcc cannot cache the result of the output from > > the asm as if it was a pure function. > > The store into buf[] dependency doesn't stop this, after unrolling: > > asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base)); > buf64[i++] = x; > asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base)); > buf64[i++] = x; > > from being reordered as this > > asm volatile ("ldrd\t%0, [%1]" : "=&r" (x2) : "r" (io_base)); > asm volatile ("ldrd\t%0, [%1]" : "=&r" (x1) : "r" (io_base)); > buf64[i++] = x1; > buf64[i++] = x2; > > because the asm doesn't depend on memory, just register inputs and > outputs; I disagree. The volatile tells gcc that the asm has side effects, and therefore 1) they can't be optimized away, and 2) can't be swapped with regards to each other like you do in your example. > 'Q' > A memory reference where the exact address is in a single > register (''m'' is preferable for 'asm' statements) Hmmm... Is this something new? I must have missed it before. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |