Re: s390: Avoid CAS boolean output inefficiency

["Ulrich Weigand" <uweigand@de.ibm.com>]

Richard Henderson wrote:
> On 08/07/2012 10:02 AM, Ulrich Weigand wrote:
> > The following patch changes the builtin expander to pass a MEM oldval
> > as-is to the back-end expander, so that the back-end can move the
> > store to before the CC operation.  With that patch I'm also seeing
> > all the IPMs disappear.
> ...
> > What do you think about this solution?  It has the advantage that
> > we still get the same xor code if we actually do need the ipm ...
> 
> I'm ok with that patch.

Thanks!  I've checked in the following version.
Tested on s390x-ibm-linux with no regressions.

Bye,
Ulrich

ChangeLog:

	* builtins.c (expand_builtin_atomic_compare_exchange): Pass old
	value operand as MEM to expand_atomic_compare_and_swap.

	* config/s390/s390.md ("atomic_compare_and_swap<mode>"): Accept
	nonimmediate_operand for old value; generate load and store if
	needed.
	* config/s390/s390.c (s390_expand_cs_hqi): Accept any operand
	as vtarget.

Index: gcc/builtins.c
===================================================================
*** gcc/builtins.c	(revision 190226)
--- gcc/builtins.c	(working copy)
*************** expand_builtin_atomic_compare_exchange (
*** 5376,5381 ****
--- 5376,5382 ----
  
    expect = expand_normal (CALL_EXPR_ARG (exp, 1));
    expect = convert_memory_address (Pmode, expect);
+   expect = gen_rtx_MEM (mode, expect);
    desired = expand_expr_force_mode (CALL_EXPR_ARG (exp, 2), mode);
  
    weak = CALL_EXPR_ARG (exp, 3);
*************** expand_builtin_atomic_compare_exchange (
*** 5383,5396 ****
    if (host_integerp (weak, 0) && tree_low_cst (weak, 0) != 0)
      is_weak = true;
  
!   oldval = copy_to_reg (gen_rtx_MEM (mode, expect));
! 
    if (!expand_atomic_compare_and_swap ((target == const0_rtx ? NULL : &target),
  				       &oldval, mem, oldval, desired,
  				       is_weak, success, failure))
      return NULL_RTX;
  
!   emit_move_insn (gen_rtx_MEM (mode, expect), oldval);
    return target;
  }
  
--- 5384,5398 ----
    if (host_integerp (weak, 0) && tree_low_cst (weak, 0) != 0)
      is_weak = true;
  
!   oldval = expect;
    if (!expand_atomic_compare_and_swap ((target == const0_rtx ? NULL : &target),
  				       &oldval, mem, oldval, desired,
  				       is_weak, success, failure))
      return NULL_RTX;
  
!   if (oldval != expect)
!     emit_move_insn (expect, oldval);
! 
    return target;
  }
  
Index: gcc/config/s390/s390.c
===================================================================
*** gcc/config/s390/s390.c	(revision 190226)
--- gcc/config/s390/s390.c	(working copy)
*************** s390_expand_cs_hqi (enum machine_mode mo
*** 4825,4831 ****
    rtx res = gen_reg_rtx (SImode);
    rtx csloop = NULL, csend = NULL;
  
-   gcc_assert (register_operand (vtarget, VOIDmode));
    gcc_assert (MEM_P (mem));
  
    init_alignment_context (&ac, mem, mode);
--- 4825,4830 ----
Index: gcc/config/s390/s390.md
===================================================================
*** gcc/config/s390/s390.md	(revision 190226)
--- gcc/config/s390/s390.md	(working copy)
***************
*** 8870,8876 ****
  
  (define_expand "atomic_compare_and_swap<mode>"
    [(match_operand:SI 0 "register_operand")	;; bool success output
!    (match_operand:DGPR 1 "register_operand")	;; oldval output
     (match_operand:DGPR 2 "memory_operand")	;; memory
     (match_operand:DGPR 3 "register_operand")	;; expected intput
     (match_operand:DGPR 4 "register_operand")	;; newval intput
--- 8870,8876 ----
  
  (define_expand "atomic_compare_and_swap<mode>"
    [(match_operand:SI 0 "register_operand")	;; bool success output
!    (match_operand:DGPR 1 "nonimmediate_operand");; oldval output
     (match_operand:DGPR 2 "memory_operand")	;; memory
     (match_operand:DGPR 3 "register_operand")	;; expected intput
     (match_operand:DGPR 4 "register_operand")	;; newval intput
***************
*** 8879,8887 ****
     (match_operand:SI 7 "const_int_operand")]	;; failure model
    ""
  {
!   rtx cc, cmp;
    emit_insn (gen_atomic_compare_and_swap<mode>_internal
! 	     (operands[1], operands[2], operands[3], operands[4]));
    cc = gen_rtx_REG (CCZ1mode, CC_REGNUM);
    cmp = gen_rtx_EQ (SImode, cc, const0_rtx);
    emit_insn (gen_cstorecc4 (operands[0], cmp, cc, const0_rtx));
--- 8879,8900 ----
     (match_operand:SI 7 "const_int_operand")]	;; failure model
    ""
  {
!   rtx cc, cmp, output = operands[1];
! 
!   if (!register_operand (output, <MODE>mode))
!     output = gen_reg_rtx (<MODE>mode);
! 
    emit_insn (gen_atomic_compare_and_swap<mode>_internal
! 	     (output, operands[2], operands[3], operands[4]));
! 
!   /* We deliberately accept non-register operands in the predicate
!      to ensure the write back to the output operand happens *before*
!      the store-flags code below.  This makes it easier for combine
!      to merge the store-flags code with a potential test-and-branch
!      pattern following (immediately!) afterwards.  */
!   if (output != operands[1])
!     emit_move_insn (operands[1], output);
! 
    cc = gen_rtx_REG (CCZ1mode, CC_REGNUM);
    cmp = gen_rtx_EQ (SImode, cc, const0_rtx);
    emit_insn (gen_cstorecc4 (operands[0], cmp, cc, const0_rtx));
***************
*** 8890,8896 ****
  
  (define_expand "atomic_compare_and_swap<mode>"
    [(match_operand:SI 0 "register_operand")	;; bool success output
!    (match_operand:HQI 1 "register_operand")	;; oldval output
     (match_operand:HQI 2 "memory_operand")	;; memory
     (match_operand:HQI 3 "general_operand")	;; expected intput
     (match_operand:HQI 4 "general_operand")	;; newval intput
--- 8903,8909 ----
  
  (define_expand "atomic_compare_and_swap<mode>"
    [(match_operand:SI 0 "register_operand")	;; bool success output
!    (match_operand:HQI 1 "nonimmediate_operand")	;; oldval output
     (match_operand:HQI 2 "memory_operand")	;; memory
     (match_operand:HQI 3 "general_operand")	;; expected intput
     (match_operand:HQI 4 "general_operand")	;; newval intput

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com