Re: [PATCH, s390, sparc, xtensa, ia64, rs6000, mips]: Unify memory_barrier patterns

[Richard Sandiford <rdsandiford at googlemail dot com>]

Uros Bizjak <ubizjak@gmail.com> writes:
> Now that UNSPEC_VOLATILEs represent a scheduling barrier for all 
> instructions, including insns with memory operands only, we can unify 
> "memory_barrier" insn pattern between all targets:
>
> - define as unspec with volatile memory operands - no need for 
> unspec_volatile anymore
> - remove one level of mem:BLK from (mem:BLK (mem:BLK (scratch...)) input 
> operand definition
> - use Pmode for scratch register
>
> Memory barrier should now be defined as:
>
> --cut here--
> (define_expand "memory_barrier"
>   [(set (match_dup 0)
>     (unspec:BLK [(match_dup 0)] UNSPEC_MB))]
>   ""
> {
>   operands[0] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
>   MEM_VOLATILE_P (operands[0]) = 1;
> })
>
> (define_insn "*memory_barrier"
>   [(set (match_operand:BLK 0 "" "")
>     (unspec:BLK [(match_dup 0)] UNSPEC_MB))]
>   ""
>   "mb"
>   [(set_attr "type" "mb")])
> --cut here--
>
> 2009-01-02  Uros Bizjak  <ubizjak@gmail.com>
>
>     * config/s390/s390.md (UNSPEC_MB): Rename from UNSPECV_MB.
>     (memory_barrier): Expand as unspec instead of unspec_volatile.
>     Remove mem:BLK from insn operands.  Use Pmode scratch register.
>     (*memory_barrier): Define as unspec instead of unspec_volatile.
>     Use (match_dup 0) as input operand.
>
>     * config/sparc/sparc.md (UNSPEC_MEMBAR): Rename from UNSPECV_MEMBAR.
>     * config/sparc/sync.md (memory_barrier): Expand as unspec instead of
>     unspec_volatile.  Remove mem:BLK from insn operands.  Use Pmode
>     scratch register.  Remove operand 1.
>     (*stbar): Define as unspec instead of unspec_volatile.
>     Use (match_dup 0) as input operand, remove (const_int 8).
>     (*membar): Define as unspec instead of unspec_volatile.
>     Use (match_dup 0) as input operand, remove input operand 2.
>
>     * config/xtensa/xtensa.md (UNSPEC_MEMW): Rename from UNSPECV_MEMW.
>     (memory_barrier): Expand as unspec instead of unspec_volatile.
>     Remove mem:BLK from insn operands.  Use Pmode scratch register.
>     (*memory_barrier): Define as unspec instead of unspec_volatile.
>     Use (match_dup 0) as input operand.
>
>     * config/ia64/sync.md (memory_barrier): Redefine as expander pattern.
>     Remove mem:BLK from insn operands.  Use Pmode scratch register.
>     Set volatile flag on operand 0.
>     (*memory_barrier): New insn pattern.
>
>     * config/rs6000/sync.md (memory_barrier): Remove mem:BLK from
>     insn operands.
>     (*memory_barrier): Use (match_dup 0) as input operand.
>
>     * config/mips/sync.md (memory_barrier): Redefine as expander pattern.
>     Remove mem:BLK from insn operands.  Use Pmode scratch register.
>     Set volatile flag on operand 0.
>     (*memory_barrier): New insn pattern.
>
>     * config/alpha/sync.md (*memory_barrier): Rename from *mb_internal.
>
> Patch was compile tested on all affected targets. Unless target 
> maintainers have objections, I plan to commit this patch as obvious in a 
> couple of days.

Thanks for doing this.  (You forgot to attach the patch btw.)

It's a very minor thing, but I notice the changelog and patch didn't
agree on the name for the new MIPS insn.  The changelog version is
more consistent with other MIPS patterns, so I went ahead and made
the code match.

Richard


gcc/
	* config/mips/sync.md (*mb_barrier): Rename to...
	(*memory_barrier): ...this.

Index: gcc/config/mips/sync.md
===================================================================
--- gcc/config/mips/sync.md	(rÃvision 143059)
+++ gcc/config/mips/sync.md	(copie de travail)
@@ -36,7 +36,7 @@ (define_expand "memory_barrier"
   MEM_VOLATILE_P (operands[0]) = 1;
 })
 
-(define_insn "*mb_internal"
+(define_insn "*memory_barrier"
   [(set (match_operand:BLK 0 "" "")
 	(unspec:BLK [(match_dup 0)] UNSPEC_MEMORY_BARRIER))]
   "GENERATE_SYNC"