| Message ID | 4ED17EB4.4070807@twiddle.net |
|---|---|
| State | New |
| Headers | show |
On Sat, 26 Nov 2011, Richard Henderson wrote: > The m68k-linux failure for the various omp atomic tests > is due to the fact that BIGGEST_ALIGNMENT is 16 bits on > that platform. I think it's pretty reasonable to assume > that if something is aligned to BIGGEST_ALIGNEMENT, then > it can be considered "aligned". BIGGEST_ALIGNMENT means aligned enough for normal access, but not necessarily for atomic access. This particular fix wouldn't do it for CRIS, for example, were BIGGEST_ALIGNMENT is 8 (bits), but an atomic access (inside the ll/sc sequence) such as for a futex requires not straddling a page boundary, i.e. "natural" alignment. Not that OMP support is imminent or critical for cris-linux or anything, but can we have a new macro? brgds, H-P
On 11/28/2011 08:49 PM, Hans-Peter Nilsson wrote: > On Sat, 26 Nov 2011, Richard Henderson wrote: >> The m68k-linux failure for the various omp atomic tests >> is due to the fact that BIGGEST_ALIGNMENT is 16 bits on >> that platform. I think it's pretty reasonable to assume >> that if something is aligned to BIGGEST_ALIGNEMENT, then >> it can be considered "aligned". > > BIGGEST_ALIGNMENT means aligned enough for normal access, but > not necessarily for atomic access. If that's true, then you'll have problems applying any of these functions without additional source-code level alignment, everywhere. > Not that OMP support is imminent or critical for cris-linux or > anything, but can we have a new macro? I'm not sure what you're suggesting that the macro actually do. r~
On Tue, 29 Nov 2011, Richard Henderson wrote: > On 11/28/2011 08:49 PM, Hans-Peter Nilsson wrote: > > On Sat, 26 Nov 2011, Richard Henderson wrote: > >> The m68k-linux failure for the various omp atomic tests > >> is due to the fact that BIGGEST_ALIGNMENT is 16 bits on > >> that platform. I think it's pretty reasonable to assume > >> that if something is aligned to BIGGEST_ALIGNEMENT, then > >> it can be considered "aligned". > > > > BIGGEST_ALIGNMENT means aligned enough for normal access, but > > not necessarily for atomic access. > > If that's true, It's what that macro meant up until gcc started to be atomicity-aware at this level, as implied by "when violated, may cause a fault". Changing it to higher makes gcc do all stupid things when accessing structure members with lower alignment so I can't do that, it violates the byte-aligment ABI. > then you'll have problems applying any of these > functions without additional source-code level alignment, everywhere. There has to be a type that matches the (let's call it) ATOMIC_ALIGNMENT yes, is that what you mean by "any of these functions"? > > Not that OMP support is imminent or critical for cris-linux or > > anything, but can we have a new macro? > > I'm not sure what you're suggesting that the macro actually do. Tell proper aligmnent for atomic access, defaulting to (say) natural aligmnent. brgds, H-P
On Tue, 29 Nov 2011, Hans-Peter Nilsson wrote: > On Tue, 29 Nov 2011, Richard Henderson wrote: > > On 11/28/2011 08:49 PM, Hans-Peter Nilsson wrote: > > > On Sat, 26 Nov 2011, Richard Henderson wrote: > > >> The m68k-linux failure for the various omp atomic tests > > >> is due to the fact that BIGGEST_ALIGNMENT is 16 bits on > > >> that platform. I think it's pretty reasonable to assume > > >> that if something is aligned to BIGGEST_ALIGNEMENT, then > > >> it can be considered "aligned". > > > > > > BIGGEST_ALIGNMENT means aligned enough for normal access, but > > > not necessarily for atomic access. > > > > If that's true, > > It's what that macro meant up until gcc started to be > atomicity-aware at this level, as implied by "when violated, may > cause a fault". Changing it to higher makes gcc do all stupid > things when accessing structure members with lower alignment so > I can't do that, it violates the byte-aligment ABI. > > > then you'll have problems applying any of these > > functions without additional source-code level alignment, everywhere. > > There has to be a type that matches the (let's call it) > ATOMIC_ALIGNMENT yes, is that what you mean by "any of these > functions"? Oh, on second reading I see you probably mean I have to make sure the atomic types are aligned in the library, by e.g. attaching __attribute__ ((__aligned__)). Sure: the reply to this change in the gut of gcc is however more important to make sure it's not cast in stone and copied to other places that I'll only find the hard way. BTW, on the topic, I cringe whenever I see futexes expressed as plain "int", they absolutely have to have at least natural alignment which is not always true e.g. in structs. People, please keep the atomic types target-overridable in libraries. > > > Not that OMP support is imminent or critical for cris-linux or > > > anything, but can we have a new macro? > > > > I'm not sure what you're suggesting that the macro actually do. > > Tell proper aligmnent for atomic access, defaulting to (say) > natural aligmnent. > > brgds, H-P >
Hans-Peter Nilsson writes: > BTW, on the topic, I cringe whenever I > see futexes expressed as plain "int", they absolutely have to > have at least natural alignment which is not always true e.g. in > structs. People, please keep the atomic types > target-overridable in libraries. +1 for m68k-linux, where plain "int" only has 16-bit alignment (by SW convention, Linux-capable HW tolerates 8-bit alignment), but futexes must be 32-bit aligned (or at least not cross page boundaries).
On Fri, 2 Dec 2011, Mikael Pettersson wrote: > but futexes must be 32-bit aligned (or at least not cross page > boundaries). Don't mix up futexes with hardware-mandated atomic alignment (except that preferably the letter should not be more strict). Linux futexes must be 32-bit aligned *for all architectures*. Linux uses the two low bits for its own purposes, when it stores futex addresses internally (or something to that effect; it was a while ago I looked at this, around Feb 2009 alt. 2.6.27-ish). brgds, H-P
On Sat, Nov 26, 2011 at 04:05:08PM -0800, Richard Henderson wrote: > The m68k-linux failure for the various omp atomic tests > is due to the fact that BIGGEST_ALIGNMENT is 16 bits on > that platform. I think it's pretty reasonable to assume > that if something is aligned to BIGGEST_ALIGNEMENT, then > it can be considered "aligned". > > Tested on x86_64-linux and m68k-linux cross. > > > r~ > * omp-low.c (expand_omp_atomic): Assume anything aligned to > BIGGEST_ALIGNMENT is aligned. That broke the atomic-2.c libgomp testcase on s390x. We have BIGGEST_ALIGNMENT of 64. A 128 bit long double does not need to be aligned better than 64 bit in memory. However, the 128bit compare and swap instruction we have requires the operands to be naturally aligned. In the testcase a compare and swap double instruction is used on a long double value which is only 8 byte aligned in memory. This seem to have caused problems on CRIS as well. The proposed solution was to force the alignment of the types using that aligned attribute. While this is a good idea anyway in order to get the proper hardware instructions, I think omp-low should be able to pick a fallback solution if necessary. Otherwise, we will silently insert bugs which are difficult to find. The atomic-2 testcase for example succeeds very often since the long double happens to be naturally aligned in a lot of cases. So to my understanding the change above is incorrect. Bye, -Andreas-
diff --git a/gcc/omp-low.c b/gcc/omp-low.c index a4bfb84..4e1c2ba 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -5501,7 +5501,9 @@ expand_omp_atomic (struct omp_region *region) unsigned int align = TYPE_ALIGN_UNIT (type); /* __sync builtins require strict data alignment. */ - if (exact_log2 (align) >= index) + /* ??? Assume BIGGEST_ALIGNMENT *is* aligned. */ + if (exact_log2 (align) >= index + || align * BITS_PER_UNIT >= BIGGEST_ALIGNMENT) { /* Atomic load. */ if (loaded_val == stored_val
The m68k-linux failure for the various omp atomic tests is due to the fact that BIGGEST_ALIGNMENT is 16 bits on that platform. I think it's pretty reasonable to assume that if something is aligned to BIGGEST_ALIGNEMENT, then it can be considered "aligned". Tested on x86_64-linux and m68k-linux cross. r~ * omp-low.c (expand_omp_atomic): Assume anything aligned to BIGGEST_ALIGNMENT is aligned.