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 <i...@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);
> })
>
> ;;
>
