Committed: CRIS: fix broken config/cpu/cris/atomicity.h. Needed for 3.4 too.

Hans-Peter Nilsson hans-peter.nilsson@axis.com
Mon Mar 22 01:40:00 GMT 2004


The reorg in libstdc++ 2004-02-25 that changed the semantics of
config/*/atomicity.h and broke out atomic_word.h broke cris-*.
I noticed and fixed locally (after some confusion regarding the
.h-as-.cc change) but haven't taken the time to communicate this
matter until now.  It also became critical as the change was
imported into the 3.4 branch.

Excuse the following venting:
Calling it atomicity.h and changing configury to compile it as
atomicity.cc is just plain wrong and confusing.  I hope that
will change back.  The prototype for the functions were also
restricted and changed with the introduction of bits/atomicity.h
(target no longer control over the prototype and the function
can't be inline), but that wasn't adjusted for in cris/atomicity.h.
Or perhaps it was but the unbreaking change was not committed
for CRIS; it seems to be have been mentioned in the ChangeLog.
The documentation in porting.texi is also out of sync after this
change.  No patch from me, because I'd rather go back to what
was documented.

I also added back the memory clobber removed last December.  (It
doesn't matter *for current use of atomicity.h* while it's
actually atomic.cc or until inter-compilation-unit-processing
looks inside libraries; LLVM?)  It must not be removed again for
this target, I expressly forbid that.  The argument for removing
the memory clobber everywhere was IMO insufficient: I don't
agree with the reasoning that to omit them is safe because of
current use.  Any performance improvement seen from removing the
memory clobbers, where it would possibly be "safe" to not guard
against memory accesses being scheduled or moved over the asm,
are *vastly* overshadowed by the risk of unsafe use being
introduced (it's obvious that libstdc++-v3 is still very much a
moving target), particularly using those "tempting" function
names and the presence of "volatile" qualifiers.  (Why were
those added, *really*?)  If you don't understand what I'm
talking about, never remove a memory clobber and always add them
to asms implementing synchronicity primitives.  If you do
understand, add them anyways, it's worth the performance cost
you think there is.
End of venting.

Committed on trunk.  Will open PR for the 3.4 branch.

	* config/cpu/cris/atomicity.h (__atomic_add): Remove "static
	inline" and attribute-unused.  Qualify parameter __mem with
	"volatile".
	(__exchange_and_add): Ditto.  Add back memory clobber to asm.

Index: atomicity.h
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/config/cpu/cris/atomicity.h,v
retrieving revision 1.5
diff -p -c -r1.5 atomicity.h
*** atomicity.h	27 Feb 2004 00:49:46 -0000	1.5
--- atomicity.h	22 Mar 2004 00:42:35 -0000
***************
*** 31,39 ****
  
  namespace __gnu_cxx
  {
!   static inline _Atomic_word
!   __attribute__ ((__unused__))
!   __exchange_and_add(_Atomic_word* __mem, int __val)
    {
      int __tmp;
      _Atomic_word __result;
--- 31,38 ----
  
  namespace __gnu_cxx
  {
!   _Atomic_word
!   __exchange_and_add(volatile _Atomic_word* __mem, int __val)
    {
      int __tmp;
      _Atomic_word __result;
*************** namespace __gnu_cxx
*** 49,57 ****
  			" bwf 0b		\n"
  			" clearf		\n"
  			:  "=&r" (__result), "=m" (*__mem), "=&r" (__tmp)
! 			: "r" (__mem), "g" (__val), "m" (*__mem));
  #else
!     __asm__ __volatile__ (" move $ccr,$r9		\n"
  			" di			\n"
  			" move.d %4,%2		\n"
  			" move.d [%3],%0	\n"
--- 48,59 ----
  			" bwf 0b		\n"
  			" clearf		\n"
  			:  "=&r" (__result), "=m" (*__mem), "=&r" (__tmp)
! 			: "r" (__mem), "g" (__val), "m" (*__mem)
! 			/* The memory clobber must stay, regardless of
! 			   current uses of this function.  */
! 			: "memory");
  #else
!     __asm__ __volatile__ (" move $ccr,$r9	\n"
  			" di			\n"
  			" move.d %4,%2		\n"
  			" move.d [%3],%0	\n"
*************** namespace __gnu_cxx
*** 60,73 ****
  			" move $r9,$ccr		\n"
  			:  "=&r" (__result), "=m" (*__mem), "=&r" (__tmp)
  			: "r" (__mem), "g" (__val), "m" (*__mem)
! 			: "r9");
  #endif
  
      return __result;
    }
  
    void
!   __attribute__ ((__unused__))
!   __atomic_add(_Atomic_word* __mem, int __val)
    { __exchange_and_add(__mem, __val); }
  } // namespace __gnu_cxx
--- 62,77 ----
  			" move $r9,$ccr		\n"
  			:  "=&r" (__result), "=m" (*__mem), "=&r" (__tmp)
  			: "r" (__mem), "g" (__val), "m" (*__mem)
! 			: "r9",
! 			  /* The memory clobber must stay, regardless of
! 			     current uses of this function.  */
! 			  "memory");
  #endif
  
      return __result;
    }
  
    void
!   __atomic_add(volatile _Atomic_word* __mem, int __val)
    { __exchange_and_add(__mem, __val); }
  } // namespace __gnu_cxx

brgds, H-P



More information about the Gcc-patches mailing list