Re: [RFC / CFT] PR c++/66192 - Remove TARGET_RELAXED_ORDERING and use load acquires.

[Ramana Radhakrishnan <ramana dot radhakrishnan at foss dot arm dot com>]

On 22/05/15 18:36, Jason Merrill wrote:
> On 05/22/2015 11:23 AM, Ramana Radhakrishnan wrote:
> > On 22/05/15 15:28, Jason Merrill wrote:
> > > I do notice that get_guard_bits after build_atomic_load just won't work
> > > on non-ARM targets, as it ends up trying to take the address of a value.
> >
> > So on powerpc where targetm.guard_mask_bit is false - this is what I see.
> >
> >    &(long long int) __atomic_load_8 (&_ZGVZ1fvE1p, 2)
>
> This is the bit that doesn't make sense to me.  __atomic_load_8 returns
> a value; what does it mean to take its address?  If we're going to load
> more than just the first byte, we should mask off the rest rather than
> trying to mess with its address.
>
> It also seems unnecessary to load 8 bytes on any target; we could add a
> function to optabs.c that returns the smallest mode for which there's
> atomic load support?
>
> Jason


Right, taken all comments on-board - thanks to those who tested the 
original patch, here's v2 of the patch currently bootstrapped and tested 
only on aarch64-none-linux-gnu

This patch -

- Turns build_atomic_load into build_atomic_load_byte in order
  to do an atomic load of 1 byte instead of a full word atomic load.
- Restructures get_guard_cond to decide whether to use an atomic load
  or a standard load depending on whether code generated will be in
  a multi-threaded context or not.
- Adjusts all callers of get_guard_cond accordingly.

One of the bits of fallout that I've observed in my testing and that I'm 
not sure about what to do is that on *bare-metal* arm-none-eabi targets 
we still put out calls to __sync_synchronize on architecture versions 
that do not have a barrier instruction which will result in a link error.

While it is tempting to take the easy approach of not putting out the 
call, I suspect in practice a number of users of the bare-metal tools 
use these for their own RTOS's and other micro-OS's. Thus generating 
barriers at higher architecture levels and not generating barriers at 
lower architecture levels appears to be a bit dangerous especially on 
architectures where there is backwards compatibility (i.e. 
-mcpu=arm7tdmi on standard user code is still expected to generate code 
that works on a core that conforms to a later architecture revision).

I am considering leaving this in the ARM backend to force people to 
think what they want to do about thread safety with statics and C++ on 
bare-metal systems. If they really do not want thread safety they can 
well add -fno-threadsafe-statics or provide an appropriate 
implementation for __sync_synchronize on their platforms.

Any thoughts / comments ?

regards
Ramana

gcc/cp/ChangeLog:

2015-05-29  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

    PR c++/66192
    * cp-tree.h (get_guard_cond): Adjust declaration
    * decl.c (expand_static_init): Use atomic load acquire
     and adjust call to get_guard_cond.
    * decl2.c (build_atomic_load_byte): New function.
    (get_guard_cond): Handle thread_safety.
    (one_static_initialization_or_destruction): Adjust call to
    get_guard_cond.

gcc/ChangeLog:

2015-05-29  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

    PR c++/66192
    * config/alpha/alpha.c (TARGET_RELAXED_ORDERING): Likewise.
    * config/ia64/ia64.c (TARGET_RELAXED_ORDERING): Likewise.
    * config/rs6000/rs6000.c (TARGET_RELAXED_ORDERING): Likewise.
    * config/sparc/linux.h (SPARC_RELAXED_ORDERING): Likewise.
    * config/sparc/linux64.h (SPARC_RELAXED_ORDERING): Likewise.
    * config/sparc/sparc.c (TARGET_RELAXED_ORDERING): Likewise.
    * config/sparc/sparc.h (SPARC_RELAXED_ORDERING): Likewise.
    * doc/tm.texi: Regenerate.
    * doc/tm.texi.in (TARGET_RELAXED_ORDERING): Delete.
    * target.def (TARGET_RELAXED_ORDERING): Delete.

Attachment: relaxed-ordering-rewrite-v2.txt
Description: Text document