Hi Haochen,
on 2023/4/6 13:35, HAO CHEN GUI wrote:
> Hi,
> This patch removes byte reverse operation before vector integer sign
> extension on big endian. These built-ins require to sign extend the element
> of the input vector that would fall in the least significant portion of the
> result element. So both BE and LE should do the same operation and the byte
> reversion is no need. This patch fixes it. Now these built-ins have the same
> behavior on all compilers.
>
> The unnecessary expand patterns are removed and the names of insn pattern
> are set to the same style. Also the test cases are modified.
>
> The patch passed regression test on Power Linux platforms.
OK for trunk and affected active branches (as PR108812 gcc-12 and gcc-11
suffer this issue too), thanks!
BR,
Kewen
>
> Thanks
> Gui Haochen
>
> ChangeLog
> rs6000: correct vector sign extend builtins on Big Endian
>
> gcc/
> PR target/108812
> * config/rs6000/vsx.md (vsx_sign_extend_qi_<mode>): Rename to...
> (vsx_sign_extend_v16qi_<mode>): ... this.
> (vsx_sign_extend_hi_<mode>): Rename to...
> (vsx_sign_extend_v8hi_<mode>): ... this.
> (vsx_sign_extend_si_v2di): Rename to...
> (vsx_sign_extend_v4si_v2di): ... this.
> (vsignextend_qi_<mode>): Remove.
> (vsignextend_hi_<mode>): Remove.
> (vsignextend_si_v2di): Remove.
> (vsignextend_v2di_v1ti): Remove.
> (*xxspltib_<mode>_split): Replace gen_vsx_sign_extend_qi_v2di with
> gen_vsx_sign_extend_v16qi_v2di and gen_vsx_sign_extend_qi_v4si
> with gen_vsx_sign_extend_v16qi_v4si.
> * config/rs6000/rs6000.md (split for DI constant generation):
> Replace gen_vsx_sign_extend_qi_si with gen_vsx_sign_extend_v16qi_si.
> (split for HSDI constant generation): Replace gen_vsx_sign_extend_qi_di
> with gen_vsx_sign_extend_v16qi_di and gen_vsx_sign_extend_qi_si
> with gen_vsx_sign_extend_v16qi_si.
> * config/rs6000/rs6000-builtins.def (__builtin_altivec_vsignextsb2d):
> Set bif-pattern to vsx_sign_extend_v16qi_v2di.
> (__builtin_altivec_vsignextsb2w): Set bif-pattern to
> vsx_sign_extend_v16qi_v4si.
> (__builtin_altivec_visgnextsh2d): Set bif-pattern to
> vsx_sign_extend_v8hi_v2di.
> (__builtin_altivec_vsignextsh2w): Set bif-pattern to
> vsx_sign_extend_v8hi_v4si.
> (__builtin_altivec_vsignextsw2d): Set bif-pattern to
> vsx_sign_extend_si_v2di.
> (__builtin_altivec_vsignext): Set bif-pattern to
> vsx_sign_extend_v2di_v1ti.
> * config/rs6000/rs6000-builtin.cc (lxvrse_expand_builtin): Replace
> gen_vsx_sign_extend_qi_v2di with gen_vsx_sign_extend_v16qi_v2di,
> gen_vsx_sign_extend_hi_v2di with gen_vsx_sign_extend_v8hi_v2di and
> gen_vsx_sign_extend_si_v2di with gen_vsx_sign_extend_v4si_v2di.
>
> gcc/testsuite/
> PR target/108812
> * gcc.target/powerpc/p9-sign_extend-runnable.c: Set corresponding
> expected vectors for Big Endian.
> * gcc.target/powerpc/int_128bit-runnable.c: Likewise.
>
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000-builtin.cc
> b/gcc/config/rs6000/rs6000-builtin.cc
> index 90ab39dc258..c66cff17681 100644
> --- a/gcc/config/rs6000/rs6000-builtin.cc
> +++ b/gcc/config/rs6000/rs6000-builtin.cc
> @@ -2840,17 +2840,17 @@ lxvrse_expand_builtin (rtx target, insn_code icode,
> rtx *op,
> if (icode == CODE_FOR_vsx_lxvrbx)
> {
> temp1 = simplify_gen_subreg (V16QImode, tiscratch, TImode, 0);
> - emit_insn (gen_vsx_sign_extend_qi_v2di (discratch, temp1));
> + emit_insn (gen_vsx_sign_extend_v16qi_v2di (discratch, temp1));
> }
> else if (icode == CODE_FOR_vsx_lxvrhx)
> {
> temp1 = simplify_gen_subreg (V8HImode, tiscratch, TImode, 0);
> - emit_insn (gen_vsx_sign_extend_hi_v2di (discratch, temp1));
> + emit_insn (gen_vsx_sign_extend_v8hi_v2di (discratch, temp1));
> }
> else if (icode == CODE_FOR_vsx_lxvrwx)
> {
> temp1 = simplify_gen_subreg (V4SImode, tiscratch, TImode, 0);
> - emit_insn (gen_vsx_sign_extend_si_v2di (discratch, temp1));
> + emit_insn (gen_vsx_sign_extend_v4si_v2di (discratch, temp1));
> }
> else if (icode == CODE_FOR_vsx_lxvrdx)
> discratch = simplify_gen_subreg (V2DImode, tiscratch, TImode, 0);
> diff --git a/gcc/config/rs6000/rs6000-builtins.def
> b/gcc/config/rs6000/rs6000-builtins.def
> index f76f54793d7..6bfe9246a02 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -2687,19 +2687,19 @@
> VRLWNM altivec_vrlwnm {}
>
> const vsll __builtin_altivec_vsignextsb2d (vsc);
> - VSIGNEXTSB2D vsignextend_qi_v2di {}
> + VSIGNEXTSB2D vsx_sign_extend_v16qi_v2di {}
>
> const vsi __builtin_altivec_vsignextsb2w (vsc);
> - VSIGNEXTSB2W vsignextend_qi_v4si {}
> + VSIGNEXTSB2W vsx_sign_extend_v16qi_v4si {}
>
> const vsll __builtin_altivec_visgnextsh2d (vss);
> - VSIGNEXTSH2D vsignextend_hi_v2di {}
> + VSIGNEXTSH2D vsx_sign_extend_v8hi_v2di {}
>
> const vsi __builtin_altivec_vsignextsh2w (vss);
> - VSIGNEXTSH2W vsignextend_hi_v4si {}
> + VSIGNEXTSH2W vsx_sign_extend_v8hi_v4si {}
>
> const vsll __builtin_altivec_vsignextsw2d (vsi);
> - VSIGNEXTSW2D vsignextend_si_v2di {}
> + VSIGNEXTSW2D vsx_sign_extend_v4si_v2di {}
>
> const vsc __builtin_altivec_vslv (vsc, vsc);
> VSLV vslv {}
> @@ -3440,7 +3440,7 @@
> VRLQNM altivec_vrlqnm {}
>
> const vsq __builtin_altivec_vsignext (vsll);
> - VSIGNEXTSD2Q vsignextend_v2di_v1ti {}
> + VSIGNEXTSD2Q vsx_sign_extend_v2di_v1ti {}
>
> const vsc __builtin_altivec_vsldb_v16qi (vsc, vsc, const int<3>);
> VSLDB_V16QI vsldb_v16qi {}
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 6011f5bf76a..17b5cd171b1 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -7793,7 +7793,7 @@ (define_split
> rtx op0_v16qi = gen_rtx_REG (V16QImode, r);
>
> emit_insn (gen_xxspltib_v16qi (op0_v16qi, op1));
> - emit_insn (gen_vsx_sign_extend_qi_si (operands[0], op0_v16qi));
> + emit_insn (gen_vsx_sign_extend_v16qi_si (operands[0], op0_v16qi));
> DONE;
> })
>
> @@ -9746,9 +9746,9 @@ (define_split
>
> emit_insn (gen_xxspltib_v16qi (op0_v16qi, op1));
> if (<MODE>mode == DImode)
> - emit_insn (gen_vsx_sign_extend_qi_di (operands[0], op0_v16qi));
> + emit_insn (gen_vsx_sign_extend_v16qi_di (operands[0], op0_v16qi));
> else if (<MODE>mode == SImode)
> - emit_insn (gen_vsx_sign_extend_qi_si (operands[0], op0_v16qi));
> + emit_insn (gen_vsx_sign_extend_v16qi_si (operands[0], op0_v16qi));
> else if (<MODE>mode == HImode)
> {
> rtx op0_v8hi = gen_rtx_REG (V8HImode, r);
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 992fbc983be..e8259385f96 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -1158,10 +1158,10 @@ (define_insn_and_split "*xxspltib_<mode>_split"
> emit_insn (gen_xxspltib_v16qi (tmp, GEN_INT (value)));
>
> if (<MODE>mode == V2DImode)
> - emit_insn (gen_vsx_sign_extend_qi_v2di (op0, tmp));
> + emit_insn (gen_vsx_sign_extend_v16qi_v2di (op0, tmp));
>
> else if (<MODE>mode == V4SImode)
> - emit_insn (gen_vsx_sign_extend_qi_v4si (op0, tmp));
> + emit_insn (gen_vsx_sign_extend_v16qi_v4si (op0, tmp));
>
> else if (<MODE>mode == V8HImode)
> emit_insn (gen_altivec_vupkhsb (op0, tmp));
> @@ -4905,27 +4905,9 @@ (define_insn "vsx_sign_extend_v2di_v1ti"
> "vextsd2q %0,%1"
> [(set_attr "type" "vecexts")])
>
> -(define_expand "vsignextend_v2di_v1ti"
> - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v")
> - (unspec:V1TI [(match_operand:V2DI 1 "vsx_register_operand" "v")]
> - UNSPEC_VSX_SIGN_EXTEND))]
> - "TARGET_POWER10"
> -{
> - if (BYTES_BIG_ENDIAN)
> - {
> - rtx tmp = gen_reg_rtx (V2DImode);
> -
> - emit_insn (gen_altivec_vrevev2di2(tmp, operands[1]));
> - emit_insn (gen_vsx_sign_extend_v2di_v1ti(operands[0], tmp));
> - DONE;
> - }
> -
> - emit_insn (gen_vsx_sign_extend_v2di_v1ti(operands[0], operands[1]));
> -})
> -
> ;; ISA 3.0 vector extend sign support
>
> -(define_insn "vsx_sign_extend_qi_<mode>"
> +(define_insn "vsx_sign_extend_v16qi_<mode>"
> [(set (match_operand:VSINT_84 0 "vsx_register_operand" "=v")
> (unspec:VSINT_84
> [(match_operand:V16QI 1 "vsx_register_operand" "v")]
> @@ -4934,25 +4916,7 @@ (define_insn "vsx_sign_extend_qi_<mode>"
> "vextsb2<wd> %0,%1"
> [(set_attr "type" "vecexts")])
>
> -(define_expand "vsignextend_qi_<mode>"
> - [(set (match_operand:VIlong 0 "vsx_register_operand" "=v")
> - (unspec:VIlong
> - [(match_operand:V16QI 1 "vsx_register_operand" "v")]
> - UNSPEC_VSX_SIGN_EXTEND))]
> - "TARGET_P9_VECTOR"
> -{
> - if (BYTES_BIG_ENDIAN)
> - {
> - rtx tmp = gen_reg_rtx (V16QImode);
> - emit_insn (gen_altivec_vrevev16qi2(tmp, operands[1]));
> - emit_insn (gen_vsx_sign_extend_qi_<mode>(operands[0], tmp));
> - }
> - else
> - emit_insn (gen_vsx_sign_extend_qi_<mode>(operands[0], operands[1]));
> - DONE;
> -})
> -
> -(define_insn "vsx_sign_extend_hi_<mode>"
> +(define_insn "vsx_sign_extend_v8hi_<mode>"
> [(set (match_operand:VSINT_84 0 "vsx_register_operand" "=v")
> (unspec:VSINT_84
> [(match_operand:V8HI 1 "vsx_register_operand" "v")]
> @@ -4961,25 +4925,7 @@ (define_insn "vsx_sign_extend_hi_<mode>"
> "vextsh2<wd> %0,%1"
> [(set_attr "type" "vecexts")])
>
> -(define_expand "vsignextend_hi_<mode>"
> - [(set (match_operand:VIlong 0 "vsx_register_operand" "=v")
> - (unspec:VIlong
> - [(match_operand:V8HI 1 "vsx_register_operand" "v")]
> - UNSPEC_VSX_SIGN_EXTEND))]
> - "TARGET_P9_VECTOR"
> -{
> - if (BYTES_BIG_ENDIAN)
> - {
> - rtx tmp = gen_reg_rtx (V8HImode);
> - emit_insn (gen_altivec_vrevev8hi2(tmp, operands[1]));
> - emit_insn (gen_vsx_sign_extend_hi_<mode>(operands[0], tmp));
> - }
> - else
> - emit_insn (gen_vsx_sign_extend_hi_<mode>(operands[0], operands[1]));
> - DONE;
> -})
> -
> -(define_insn "vsx_sign_extend_si_v2di"
> +(define_insn "vsx_sign_extend_v4si_v2di"
> [(set (match_operand:V2DI 0 "vsx_register_operand" "=v")
> (unspec:V2DI [(match_operand:V4SI 1 "vsx_register_operand" "v")]
> UNSPEC_VSX_SIGN_EXTEND))]
> @@ -4987,24 +4933,6 @@ (define_insn "vsx_sign_extend_si_v2di"
> "vextsw2d %0,%1"
> [(set_attr "type" "vecexts")])
>
> -(define_expand "vsignextend_si_v2di"
> - [(set (match_operand:V2DI 0 "vsx_register_operand" "=v")
> - (unspec:V2DI [(match_operand:V4SI 1 "vsx_register_operand" "v")]
> - UNSPEC_VSX_SIGN_EXTEND))]
> - "TARGET_P9_VECTOR"
> -{
> - if (BYTES_BIG_ENDIAN)
> - {
> - rtx tmp = gen_reg_rtx (V4SImode);
> -
> - emit_insn (gen_altivec_vrevev4si2(tmp, operands[1]));
> - emit_insn (gen_vsx_sign_extend_si_v2di(operands[0], tmp));
> - }
> - else
> - emit_insn (gen_vsx_sign_extend_si_v2di(operands[0], operands[1]));
> - DONE;
> -})
> -
> ;; Sign extend DI to TI. We provide both GPR targets and Altivec targets on
> ;; power10. On earlier systems, the machine independent code will generate a
> ;; shift left to sign extend the 64-bit value to 128-bit.
> diff --git a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> index 1afb00262a1..68217c62325 100644
> --- a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c
> @@ -90,7 +90,11 @@ int main ()
> vec_arg1_di[0] = 1000;
> vec_arg1_di[1] = -123456;
>
> +#ifdef __BIG_ENDIAN__
> + expected_result = -123456;
> +#else
> expected_result = 1000;
> +#endif
>
> vec_result = vec_signextq (vec_arg1_di);
>
> @@ -109,7 +113,11 @@ int main ()
> vec_arg1_di[0] = -123456;
> vec_arg1_di[1] = 1000;
>
> +#ifdef __BIG_ENDIAN__
> + expected_result = 1000;
> +#else
> expected_result = -123456;
> +#endif
>
> vec_result = vec_signextq (vec_arg1_di);
>
> diff --git a/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
> b/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
> index fdcad019b96..03c0f1201e4 100644
> --- a/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
> @@ -34,7 +34,12 @@ int main ()
> /* test sign extend byte to word */
> vec_arg_qi = (vector signed char) {1, 2, 3, 4, 5, 6, 7, 8,
> -1, -2, -3, -4, -5, -6, -7, -8};
> +
> +#ifdef __BIG_ENDIAN__
> + vec_expected_wi = (vector signed int) {4, 8, -4, -8};
> +#else
> vec_expected_wi = (vector signed int) {1, 5, -1, -5};
> +#endif
>
> vec_result_wi = vec_signexti (vec_arg_qi);
>
> @@ -54,7 +59,12 @@ int main ()
> /* test sign extend byte to double */
> vec_arg_qi = (vector signed char){1, 2, 3, 4, 5, 6, 7, 8,
> -1, -2, -3, -4, -5, -6, -7, -8};
> +
> +#ifdef __BIG_ENDIAN__
> + vec_expected_di = (vector signed long long int){8, -8};
> +#else
> vec_expected_di = (vector signed long long int){1, -1};
> +#endif
>
> vec_result_di = vec_signextll(vec_arg_qi);
>
> @@ -72,7 +82,12 @@ int main ()
>
> /* test sign extend short to word */
> vec_arg_hi = (vector signed short int){1, 2, 3, 4, -1, -2, -3, -4};
> +
> +#ifdef __BIG_ENDIAN__
> + vec_expected_wi = (vector signed int){2, 4, -2, -4};
> +#else
> vec_expected_wi = (vector signed int){1, 3, -1, -3};
> +#endif
>
> vec_result_wi = vec_signexti(vec_arg_hi);
>
> @@ -90,7 +105,12 @@ int main ()
>
> /* test sign extend short to double word */
> vec_arg_hi = (vector signed short int ){1, 3, 5, 7, -1, -3, -5, -7};
> +
> +#ifdef __BIG_ENDIAN__
> + vec_expected_di = (vector signed long long int){7, -7};
> +#else
> vec_expected_di = (vector signed long long int){1, -1};
> +#endif
>
> vec_result_di = vec_signextll(vec_arg_hi);
>
> @@ -108,7 +128,12 @@ int main ()
>
> /* test sign extend word to double word */
> vec_arg_wi = (vector signed int ){1, 3, -1, -3};
> +
> +#ifdef __BIG_ENDIAN__
> + vec_expected_di = (vector signed long long int){3, -3};
> +#else
> vec_expected_di = (vector signed long long int){1, -1};
> +#endif
>
> vec_result_di = vec_signextll(vec_arg_wi);
>
