[2/3] retire mem_signal_fence pattern

Similar to mem_thread_fence issue from the patch 1/3, RTL representation of
__atomic_signal_fence must be a compiler barrier.  We have just one backend
offering this pattern, and it does not place a compiler barrier.

It does not appear useful to expand signal_fence to some kind of hardware
instruction, its documentation is wrong/outdated, and we are using it
incorrectly anyway.  So just remove the whole thing and simply emit a compiler
memory barrier in the optabs.c handler.

	* config/s390/s390.md (mem_signal_fence): Remove.
        * doc/md.texi (mem_signal_fence): Remove.
        * optabs.c (expand_mem_signal_fence): Remove uses of mem_signal_fence.
        Update comments.
        * target-insns.def (mem_signal_fence): Remove.

---
 gcc/config/s390/s390.md |  9 ---------
 gcc/doc/md.texi         | 13 -------------
 gcc/optabs.c            | 17 +++++------------
 gcc/target-insns.def    |  1 -
 4 files changed, 5 insertions(+), 35 deletions(-)

Ping (for this and patch 3/3 in the thread).

On Wed, 2 Aug 2017, Alexander Monakov wrote:

> Similar to mem_thread_fence issue from the patch 1/3, RTL representation of
> __atomic_signal_fence must be a compiler barrier.  We have just one backend
> offering this pattern, and it does not place a compiler barrier.
> 
> It does not appear useful to expand signal_fence to some kind of hardware
> instruction, its documentation is wrong/outdated, and we are using it
> incorrectly anyway.  So just remove the whole thing and simply emit a compiler
> memory barrier in the optabs.c handler.
> 
> 	* config/s390/s390.md (mem_signal_fence): Remove.
>         * doc/md.texi (mem_signal_fence): Remove.
>         * optabs.c (expand_mem_signal_fence): Remove uses of mem_signal_fence.
>         Update comments.
>         * target-insns.def (mem_signal_fence): Remove.
> 
> ---
>  gcc/config/s390/s390.md |  9 ---------
>  gcc/doc/md.texi         | 13 -------------
>  gcc/optabs.c            | 17 +++++------------
>  gcc/target-insns.def    |  1 -
>  4 files changed, 5 insertions(+), 35 deletions(-)
> 
> diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
> index d1ac0b8395d..1d63523d3b0 100644
> --- a/gcc/config/s390/s390.md
> +++ b/gcc/config/s390/s390.md
> @@ -10084,15 +10084,6 @@ (define_insn "*basr_tls"
>  ; memory barrier patterns.
>  ;
>  
> -(define_expand "mem_signal_fence"
> -  [(match_operand:SI 0 "const_int_operand")]		;; model
> -  ""
> -{
> -  /* The s390 memory model is strong enough not to require any
> -     barrier in order to synchronize a thread with itself.  */
> -  DONE;
> -})
> -
>  (define_expand "mem_thread_fence"
>    [(match_operand:SI 0 "const_int_operand")]		;; model
>    ""
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 0be161a08dc..73d0e7d83c0 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -7058,19 +7058,6 @@ If this pattern is not specified, all memory models except
>  @code{__ATOMIC_RELAXED} will result in issuing a @code{sync_synchronize}
>  barrier pattern.
>  
> -@cindex @code{mem_signal_fence@var{mode}} instruction pattern
> -@item @samp{mem_signal_fence@var{mode}}
> -This pattern emits code required to implement a signal fence with
> -memory model semantics.  Operand 0 is the memory model to be used.
> -
> -This pattern should impact the compiler optimizers the same way that
> -mem_signal_fence does, but it does not need to issue any barrier
> -instructions.
> -
> -If this pattern is not specified, all memory models except
> -@code{__ATOMIC_RELAXED} will result in issuing a @code{sync_synchronize}
> -barrier pattern.
> -
>  @cindex @code{get_thread_pointer@var{mode}} instruction pattern
>  @cindex @code{set_thread_pointer@var{mode}} instruction pattern
>  @item @samp{get_thread_pointer@var{mode}}
> diff --git a/gcc/optabs.c b/gcc/optabs.c
> index d568ca376fe..6884fd4b8aa 100644
> --- a/gcc/optabs.c
> +++ b/gcc/optabs.c
> @@ -6315,22 +6315,15 @@ expand_mem_thread_fence (enum memmodel model)
>      }
>  }
>  
> -/* This routine will either emit the mem_signal_fence pattern or issue a 
> -   sync_synchronize to generate a fence for memory model MEMMODEL.  */
> +/* Emit a signal fence with given memory model.  */
>  
>  void
>  expand_mem_signal_fence (enum memmodel model)
>  {
> -  if (targetm.have_mem_signal_fence ())
> -    emit_insn (targetm.gen_mem_signal_fence (GEN_INT (model)));
> -  else if (!is_mm_relaxed (model))
> -    {
> -      /* By default targets are coherent between a thread and the signal
> -	 handler running on the same thread.  Thus this really becomes a
> -	 compiler barrier, in that stores must not be sunk past
> -	 (or raised above) a given point.  */
> -      expand_asm_memory_barrier ();
> -    }
> +  /* No machine barrier is required to implement a signal fence, but
> +     a compiler memory barrier must be issued, except for relaxed MM.  */
> +  if (!is_mm_relaxed (model))
> +    expand_asm_memory_barrier ();
>  }
>  
>  /* This function expands the atomic load operation:
> diff --git a/gcc/target-insns.def b/gcc/target-insns.def
> index fb92f72ac29..4669439c7e1 100644
> --- a/gcc/target-insns.def
> +++ b/gcc/target-insns.def
> @@ -58,7 +58,6 @@ DEF_TARGET_INSN (indirect_jump, (rtx x0))
>  DEF_TARGET_INSN (insv, (rtx x0, rtx x1, rtx x2, rtx x3))
>  DEF_TARGET_INSN (jump, (rtx x0))
>  DEF_TARGET_INSN (load_multiple, (rtx x0, rtx x1, rtx x2))
> -DEF_TARGET_INSN (mem_signal_fence, (rtx x0))
>  DEF_TARGET_INSN (mem_thread_fence, (rtx x0))
>  DEF_TARGET_INSN (memory_barrier, (void))
>  DEF_TARGET_INSN (movstr, (rtx x0, rtx x1, rtx x2))
>

On 08/28/2017 06:05 AM, Alexander Monakov wrote:
> Ping (for this and patch 3/3 in the thread).
> 
> On Wed, 2 Aug 2017, Alexander Monakov wrote:
> 
>> Similar to mem_thread_fence issue from the patch 1/3, RTL representation of
>> __atomic_signal_fence must be a compiler barrier.  We have just one backend
>> offering this pattern, and it does not place a compiler barrier.
>>
>> It does not appear useful to expand signal_fence to some kind of hardware
>> instruction, its documentation is wrong/outdated, and we are using it
>> incorrectly anyway.  So just remove the whole thing and simply emit a compiler
>> memory barrier in the optabs.c handler.
>>
>> 	* config/s390/s390.md (mem_signal_fence): Remove.
>>         * doc/md.texi (mem_signal_fence): Remove.
>>         * optabs.c (expand_mem_signal_fence): Remove uses of mem_signal_fence.
>>         Update comments.
>>         * target-insns.def (mem_signal_fence): Remove.
This is OK.

What's the point of the delete_insns_since calls in patch #3?

jeff

On Thu, 31 Aug 2017, Jeff Law wrote:
> This is OK.
> 
> What's the point of the delete_insns_since calls in patch #3?

Deleting the first barrier when maybe_expand_insn failed.
Other functions in the file use a similar approach.

Thanks.
Alexander

On 09/01/2017 12:26 AM, Alexander Monakov wrote:
> On Thu, 31 Aug 2017, Jeff Law wrote:
>> This is OK.
>>
>> What's the point of the delete_insns_since calls in patch #3?
> 
> Deleting the first barrier when maybe_expand_insn failed.
> Other functions in the file use a similar approach.
Thanks for clarifying.  OK for the trunk.  Sorry about the wait.

Jeff