Re: [Patch AArch64] PR target/66200 - gcc / libstdc++ TLC for weak memory models.

[James Greenhalgh]

On Thu, May 21, 2015 at 09:54:19AM +0100, Ramana Radhakrishnan wrote:
> And here's an additional patch for the testsuite which was missed in the 
> original posting.
> 
> This is a testism that's testing code generation as per 
> TARGET_RELAXED_ORDERING being false and therefore needs to be adjusted 
> as attached.
> 
> Ramana
> 
> PR target/66200
> * g++.dg/abi/aarch64_guard1.C: Adjust testcase.

OK.

Thanks,
James

> diff --git a/gcc/testsuite/g++.dg/abi/aarch64_guard1.C 
> b/gcc/testsuite/g++.dg/abi/aarch64_guard1.C
> index ca1778b..e78f93c 100644
> --- a/gcc/testsuite/g++.dg/abi/aarch64_guard1.C
> +++ b/gcc/testsuite/g++.dg/abi/aarch64_guard1.C
> @@ -13,5 +13,4 @@ int *foo ()
>  }
>  
>  // { dg-final { scan-assembler _ZGVZ3foovE1x,8,8 } }
> -// { dg-final { scan-tree-dump "_ZGVZ3foovE1x & 1" "original" } }
>  // { dg-final { cleanup-tree-dump "original" } }

Re: [Patch AArch64] PR target/66200 - gcc / libstdc++ TLC for weak memory models.

[James Greenhalgh]

On Wed, May 20, 2015 at 02:58:09PM +0100, Ramana Radhakrishnan wrote:
> Hi,
> 
>   Someone privately pointed out that the ARM and AArch64 ports do not 
> define TARGET_RELAXED_ORDERING given that the architecture(s) mandates a 
> weak memory model. This patch fixes it for AArch64, the ARM patch 
> follows in due course after appropriate testing.
> 
> I will also note that we can define __test_and_acquire as well as 
> __set_and_release and I'm toying with a follow-up patch for the same.
> 
> Also it may make sense to consider changing the defaults to a safer 
> form, or indeed forcing ports to define some of this rather than 
> allowing for silent wrong code issues. However I'm not about to do so in 
> the context of this patch.
> 
> Bootstrapped and regression tested on aarch64-none-linux-gnu with no 
> regressions.
> 
> Ok to apply to trunk and all release branches ?
> 
> gcc/
> 
> PR target/66200
> 
> * config/aarch64/aarch64.c (TARGET_RELAXED_ORDERING): Define
> 
> libstdc++-v3/
> 
> PR target/66200
> 
> * configure.host (host_cpu): Add aarch64 case.
> * config/cpu/aarch64/atomic_word.h: New file

This is OK for trunk and looks serious enough for backport to the
release branches.

Though,

> +// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.

this comment had me looking for uses of TARGET_RELAXED_ORDERING as a macro
rather than a hook, and left... But fixing that (here and in the
template) is not essential for this patch, or for backporting.

Thanks,
James

> P.S.  It's interesting to note that ia64 doesn't define the barriers 
> which appear to be used in a number of other places than just the 
> constructor guard functions (probably wrongly on the assumption that one 
> doesn't need the barriers elsewhere). I suspect other architectures like 
> MIPS may also be affected by this.

> commit 414345c424fa020717c6c3083089cd987f3032db
> Author: Ramana Radhakrishnan 
> Date:   Wed May 20 13:55:44 2015 +0100
> 
> Add relaxed memory ordering cases.
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 7f0cc0d..273aa06 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -11644,6 +11644,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool load,
>  #undef TARGET_SCHED_FUSION_PRIORITY
>  #define TARGET_SCHED_FUSION_PRIORITY aarch64_sched_fusion_priority
>  
> +#undef TARGET_RELAXED_ORDERING
> +#define TARGET_RELAXED_ORDERING true
> +
>  struct gcc_target targetm = TARGET_INITIALIZER;
>  
>  #include "gt-aarch64.h"
> diff --git a/libstdc++-v3/config/cpu/aarch64/atomic_word.h 
> b/libstdc++-v3/config/cpu/aarch64/atomic_word.h
> new file mode 100644
> index 000..4afe6ed
> --- /dev/null
> +++ b/libstdc++-v3/config/cpu/aarch64/atomic_word.h
> @@ -0,0 +1,44 @@
> +// Low-level type for atomic operations -*- C++ -*-
> +
> +// Copyright (C) 2015 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library.  This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +
> +// Under Section 7 of GPL version 3, you are granted additional
> +// permissions described in the GCC Runtime Library Exception, version
> +// 3.1, as published by the Free Software Foundation.
> +
> +// You should have received a copy of the GNU General Public License and
> +// a copy of the GCC Runtime Library Exception along with this program;
> +// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> +// .
> +
> +/** @file atomic_word.h
> + *  This file is a GNU extension to the Standard C++ Library.
> + */
> +
> +#ifndef _GLIBCXX_ATOMIC_WORD_H
> +#define _GLIBCXX_ATOMIC_WORD_H   1
> +
> +
> +typedef int _Atomic_word;
> +
> +// This one prevents loads from being hoisted across the barrier;
> +// in other words, this is a Load-Load acquire barrier.
> +// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.
> +#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("dmb ishld":::"memory")
> +
> +// This one prevents stores from being sunk across the barrier; in other
> +// words, a Store-Store release barrier.
> +#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("dmb ishst":::"memory")
> +
> +#endif
> diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host
> index a349ce3..42a45d9 100644
> --- a/libstdc++-v3/configure.host
> +++ b/libstdc++-v3/configure.host
> @@ -153,6 +153,9 @@ esac
>  # Most can just use generic.
>  # THIS TABLE IS SORTED.  KEEP IT THAT WAY.
>  case "${host_cpu}" in
> +  aarch64*)
> +   

Re: [Patch AArch64] PR target/66200 - gcc / libstdc++ TLC for weak memory models.

[Ramana Radhakrishnan]

And here's an additional patch for the testsuite which was missed in the 
original posting.


This is a testism that's testing code generation as per 
TARGET_RELAXED_ORDERING being false and therefore needs to be adjusted 
as attached.


Ramana

PR target/66200
* g++.dg/abi/aarch64_guard1.C: Adjust testcase.

diff --git a/gcc/testsuite/g++.dg/abi/aarch64_guard1.C 
b/gcc/testsuite/g++.dg/abi/aarch64_guard1.C
index ca1778b..e78f93c 100644
--- a/gcc/testsuite/g++.dg/abi/aarch64_guard1.C
+++ b/gcc/testsuite/g++.dg/abi/aarch64_guard1.C
@@ -13,5 +13,4 @@ int *foo ()
 }
 
 // { dg-final { scan-assembler _ZGVZ3foovE1x,8,8 } }
-// { dg-final { scan-tree-dump "_ZGVZ3foovE1x & 1" "original" } }
 // { dg-final { cleanup-tree-dump "original" } }

[Patch AArch64] PR target/66200 - gcc / libstdc++ TLC for weak memory models.

[Ramana Radhakrishnan]

Hi,

	Someone privately pointed out that the ARM and AArch64 ports do not 
define TARGET_RELAXED_ORDERING given that the architecture(s) mandates a 
weak memory model. This patch fixes it for AArch64, the ARM patch 
follows in due course after appropriate testing.


I will also note that we can define __test_and_acquire as well as 
__set_and_release and I'm toying with a follow-up patch for the same.


Also it may make sense to consider changing the defaults to a safer 
form, or indeed forcing ports to define some of this rather than 
allowing for silent wrong code issues. However I'm not about to do so in 
the context of this patch.


Bootstrapped and regression tested on aarch64-none-linux-gnu with no 
regressions.


Ok to apply to trunk and all release branches ?

gcc/

PR target/66200

* config/aarch64/aarch64.c (TARGET_RELAXED_ORDERING): Define

libstdc++-v3/

PR target/66200

* configure.host (host_cpu): Add aarch64 case.
* config/cpu/aarch64/atomic_word.h: New file




regards
Ramana


P.S.  It's interesting to note that ia64 doesn't define the barriers 
which appear to be used in a number of other places than just the 
constructor guard functions (probably wrongly on the assumption that one 
doesn't need the barriers elsewhere). I suspect other architectures like 
MIPS may also be affected by this.
commit 414345c424fa020717c6c3083089cd987f3032db
Author: Ramana Radhakrishnan 
Date:   Wed May 20 13:55:44 2015 +0100

Add relaxed memory ordering cases.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 7f0cc0d..273aa06 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11644,6 +11644,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool load,
 #undef TARGET_SCHED_FUSION_PRIORITY
 #define TARGET_SCHED_FUSION_PRIORITY aarch64_sched_fusion_priority
 
+#undef TARGET_RELAXED_ORDERING
+#define TARGET_RELAXED_ORDERING true
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"
diff --git a/libstdc++-v3/config/cpu/aarch64/atomic_word.h 
b/libstdc++-v3/config/cpu/aarch64/atomic_word.h
new file mode 100644
index 000..4afe6ed
--- /dev/null
+++ b/libstdc++-v3/config/cpu/aarch64/atomic_word.h
@@ -0,0 +1,44 @@
+// Low-level type for atomic operations -*- C++ -*-
+
+// Copyright (C) 2015 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+// .
+
+/** @file atomic_word.h
+ *  This file is a GNU extension to the Standard C++ Library.
+ */
+
+#ifndef _GLIBCXX_ATOMIC_WORD_H
+#define _GLIBCXX_ATOMIC_WORD_H 1
+
+
+typedef int _Atomic_word;
+
+// This one prevents loads from being hoisted across the barrier;
+// in other words, this is a Load-Load acquire barrier.
+// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.
+#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("dmb ishld":::"memory")
+
+// This one prevents stores from being sunk across the barrier; in other
+// words, a Store-Store release barrier.
+#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("dmb ishst":::"memory")
+
+#endif
diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host
index a349ce3..42a45d9 100644
--- a/libstdc++-v3/configure.host
+++ b/libstdc++-v3/configure.host
@@ -153,6 +153,9 @@ esac
 # Most can just use generic.
 # THIS TABLE IS SORTED.  KEEP IT THAT WAY.
 case "${host_cpu}" in
+  aarch64*)
+atomic_word_dir=cpu/aarch64
+;;
   alpha*)
 atomic_word_dir=cpu/alpha
 ;;