[PATCH v2] IBM Z: Change vector copysign to use bitwise operations

Andreas Krebbel krebbel@linux.ibm.com
Mon Oct 12 09:48:31 GMT 2020


On 09.10.20 17:49, Ilya Leoshkevich wrote:
> Bootstrapped and regtested on s390x-redhat-linux.  OK for master?
> 
> v1: https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555782.html
> v1 -> v2: Use related_int_vector_mode.
> 
> 
> 
> The vector copysign pattern incorrectly assumes that vector
> if_then_else operates on bits, not on elements.  This can theoretically
> mislead the optimizers.  Fix by changing it to use bitwise operations,
> like commit 2930bb321794 ("PR94613: Fix vec_sel builtin for IBM Z") did
> for vec_sel builtin.
> 
> gcc/ChangeLog:
> 
> 2020-10-07  Ilya Leoshkevich  <iii@linux.ibm.com>
> 
> 	* config/s390/s390-protos.h (s390_build_signbit_mask): New
> 	function.
> 	* config/s390/s390.c (s390_contiguous_bitmask_vector_p):
> 	Bitcast the argument to an integral mode.
> 	(s390_expand_vec_init): Do not call
> 	s390_contiguous_bitmask_vector_p with a scalar argument.
> 	(s390_build_signbit_mask): New function.
> 	* config/s390/vector.md (copysign<mode>3): Use bitwise
> 	operations.

Ok. Thanks!

Andreas

> ---
>  gcc/config/s390/s390-protos.h |  1 +
>  gcc/config/s390/s390.c        | 44 ++++++++++++++++++++++++++++-------
>  gcc/config/s390/vector.md     | 28 +++++++---------------
>  3 files changed, 45 insertions(+), 28 deletions(-)
> 
> diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h
> index 6f1bc07db17..029f7289fac 100644
> --- a/gcc/config/s390/s390-protos.h
> +++ b/gcc/config/s390/s390-protos.h
> @@ -121,6 +121,7 @@ extern void s390_expand_vec_compare_cc (rtx, enum rtx_code, rtx, rtx, bool);
>  extern enum rtx_code s390_reverse_condition (machine_mode, enum rtx_code);
>  extern void s390_expand_vcond (rtx, rtx, rtx, enum rtx_code, rtx, rtx);
>  extern void s390_expand_vec_init (rtx, rtx);
> +extern rtx s390_build_signbit_mask (machine_mode);
>  extern rtx s390_return_addr_rtx (int, rtx);
>  extern rtx s390_back_chain_rtx (void);
>  extern rtx_insn *s390_emit_call (rtx, rtx, rtx, rtx);
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index 93894307d62..dbb541bbea7 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -2467,6 +2467,9 @@ s390_contiguous_bitmask_vector_p (rtx op, int *start, int *end)
>    rtx elt;
>    bool b;
>  
> +  /* Handle floats by bitcasting them to ints.  */
> +  op = gen_lowpart (related_int_vector_mode (GET_MODE (op)).require (), op);
> +
>    gcc_assert (!!start == !!end);
>    if (!const_vec_duplicate_p (op, &elt)
>        || !CONST_INT_P (elt))
> @@ -6863,15 +6866,16 @@ s390_expand_vec_init (rtx target, rtx vals)
>      }
>  
>    /* Use vector gen mask or vector gen byte mask if possible.  */
> -  if (all_same && all_const_int
> -      && (XVECEXP (vals, 0, 0) == const0_rtx
> -	  || s390_contiguous_bitmask_vector_p (XVECEXP (vals, 0, 0),
> -					       NULL, NULL)
> -	  || s390_bytemask_vector_p (XVECEXP (vals, 0, 0), NULL)))
> +  if (all_same && all_const_int)
>      {
> -      emit_insn (gen_rtx_SET (target,
> -			      gen_rtx_CONST_VECTOR (mode, XVEC (vals, 0))));
> -      return;
> +      rtx vec = gen_rtx_CONST_VECTOR (mode, XVEC (vals, 0));
> +      if (XVECEXP (vals, 0, 0) == const0_rtx
> +	  || s390_contiguous_bitmask_vector_p (vec, NULL, NULL)
> +	  || s390_bytemask_vector_p (vec, NULL))
> +	{
> +	  emit_insn (gen_rtx_SET (target, vec));
> +	  return;
> +	}
>      }
>  
>    /* Use vector replicate instructions.  vlrep/vrepi/vrep  */
> @@ -6949,6 +6953,30 @@ s390_expand_vec_init (rtx target, rtx vals)
>      }
>  }
>  
> +/* Emit a vector constant that contains 1s in each element's sign bit position
> +   and 0s in other positions.  MODE is the desired constant's mode.  */
> +extern rtx
> +s390_build_signbit_mask (machine_mode mode)
> +{
> +  /* Generate the integral element mask value.  */
> +  machine_mode inner_mode = GET_MODE_INNER (mode);
> +  int inner_bitsize = GET_MODE_BITSIZE (inner_mode);
> +  wide_int mask_val = wi::set_bit_in_zero (inner_bitsize - 1, inner_bitsize);
> +
> +  /* Emit the element mask rtx.  Use gen_lowpart in order to cast the integral
> +     value to the desired mode.  */
> +  machine_mode int_mode = related_int_vector_mode (mode).require ();
> +  rtx mask = immed_wide_int_const (mask_val, GET_MODE_INNER (int_mode));
> +  mask = gen_lowpart (inner_mode, mask);
> +
> +  /* Emit the vector mask rtx by mode the element mask rtx.  */
> +  int nunits = GET_MODE_NUNITS (mode);
> +  rtvec v = rtvec_alloc (nunits);
> +  for (int i = 0; i < nunits; i++)
> +    RTVEC_ELT (v, i) = mask;
> +  return gen_rtx_CONST_VECTOR (mode, v);
> +}
> +
>  /* Structure to hold the initial parameters for a compare_and_swap operation
>     in HImode and QImode.  */
>  
> diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
> index 2573b7d980a..e9332bad0fd 100644
> --- a/gcc/config/s390/vector.md
> +++ b/gcc/config/s390/vector.md
> @@ -1425,28 +1425,16 @@ (define_insn_and_split "*sminv2df3_vx"
>  
>  ; Vector copysign, implement using vector select
>  (define_expand "copysign<mode>3"
> -  [(set (match_operand:VFT 0 "register_operand" "")
> -	(if_then_else:VFT
> -	 (eq (match_dup 3)
> -	     (match_dup 4))
> -	 (match_operand:VFT 1 "register_operand"  "")
> -	 (match_operand:VFT 2 "register_operand"  "")))]
> +  [(set (match_operand:VFT            0 "register_operand" "")
> +	(ior:VFT
> +	 (and:VFT (match_operand:VFT  2 "register_operand" "")
> +		  (match_dup 3))
> +	 (and:VFT (not:VFT (match_dup 3))
> +		  (match_operand:VFT  1 "register_operand" ""))))]
>    "TARGET_VX"
>  {
> -  int sz = GET_MODE_BITSIZE (GET_MODE_INNER (<MODE>mode));
> -  int prec = GET_MODE_PRECISION (GET_MODE_INNER (<tointvec>mode));
> -  wide_int mask_val = wi::shwi (1l << (sz - 1), prec);
> -
> -  rtx mask = gen_reg_rtx (<tointvec>mode);
> -
> -  int nunits = GET_MODE_NUNITS (<tointvec>mode);
> -  rtvec v = rtvec_alloc (nunits);
> -  for (int i = 0; i < nunits; i++)
> -    RTVEC_ELT (v, i) = GEN_INT (mask_val.to_shwi ());
> -
> -  mask = gen_rtx_CONST_VECTOR (<tointvec>mode, v);
> -  operands[3] = force_reg (<tointvec>mode, mask);
> -  operands[4] = CONST0_RTX (<tointvec>mode);
> +  rtx mask = s390_build_signbit_mask (<MODE>mode);
> +  operands[3] = force_reg (<MODE>mode, mask);
>  })
>  
>  ;;
> 



More information about the Gcc-patches mailing list