| Message ID | b5116971c58e360bce4cc63436a4e2e9e5d90f85.camel@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000, 128-bit Binary Integer Operations | expand |
On Tue, 2020-08-11 at 12:23 -0700, Carl Love wrote: > Segher, Will: > > Patch 4 adds 128-bit integer shift instruction support. I suggest having a few more words here to better describe what this patch is doing. i.e. This is adding the VEC_I128 iterator which contains the V1TI and TI types, and modifying existing define_insns to add handling for the VEC_I128 iterator. > > Carl Love > > --------------------------------------------------------- > Test 128-bit shifts for just the int128 type. > > gcc/ChangeLog > > 2020-08-10 Carl Love <cel@us.ibm.com> > * config/rs6000/altivec.md (altivec_vslq, altivec_vsrq): Add mode > VEC_I128. Maybe also rename to altivec_vslq_<mode> and altivec_vsrq_<mode>. > * config/rs6000/vector.md (VEC_I128): New mode iterator. ok > (vashlv1ti3): Change to vashl<mode>3, mode VEC_I128. > (vlshrv1ti3): Change to vlshr<mode>3, mode VEC_I128. > * config/rs6000/vsx.md (UNSPEC_XXSWAPD_V1TI): Change to > UNSPEC_XXSWAPD_VEC_I128. s/Change/Rename/ > (xxswapd_v1ti): Change to xxswapd_<mode>, mode VEC_I128. > > gcc/testsuite/ChangeLog > > 2020-08-10 Carl Love <cel@us.ibm.com> > gcc.target/powerpc/int_128bit-runnable.c: Add shift_right, shift_left > tests. > --- > gcc/config/rs6000/altivec.md | 16 +++++------ > gcc/config/rs6000/vector.md | 27 ++++++++++--------- > gcc/config/rs6000/vsx.md | 14 +++++----- > .../gcc.target/powerpc/int_128bit-runnable.c | 24 +++++++++++++++-- > 4 files changed, 52 insertions(+), 29 deletions(-) > > diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md > index 2763d920828..cba39852070 100644 > --- a/gcc/config/rs6000/altivec.md > +++ b/gcc/config/rs6000/altivec.md > @@ -2219,10 +2219,10 @@ > "vsl<VI_char> %0,%1,%2" > [(set_attr "type" "vecsimple")]) > > -(define_insn "altivec_vslq" > - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") > - (ashift:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") > - (match_operand:V1TI 2 "vsx_register_operand" "v")))] > +(define_insn "altivec_vslq_<mode>" > + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") > + (ashift:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand" "v") > + (match_operand:VEC_I128 2 "vsx_register_operand" "v")))] > "TARGET_TI_VECTOR_OPS" > /* Shift amount in needs to be in bits[57:63] of 128-bit operand. */ > "vslq %0,%1,%2" ok > @@ -2236,10 +2236,10 @@ > "vsr<VI_char> %0,%1,%2" > [(set_attr "type" "vecsimple")]) > > -(define_insn "altivec_vsrq" > - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") > - (lshiftrt:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") > - (match_operand:V1TI 2 "vsx_register_operand" "v")))] > +(define_insn "altivec_vsrq_<mode>" > + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") > + (lshiftrt:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand" "v") > + (match_operand:VEC_I128 2 "vsx_register_operand" "v")))] > "TARGET_TI_VECTOR_OPS" > /* Shift amount in needs to be in bits[57:63] of 128-bit operand. */ > "vsrq %0,%1,%2" ok > diff --git a/gcc/config/rs6000/vector.md b/gcc/config/rs6000/vector.md > index 2deff282076..682aabc4657 100644 > --- a/gcc/config/rs6000/vector.md > +++ b/gcc/config/rs6000/vector.md > @@ -26,6 +26,9 @@ > ;; Vector int modes > (define_mode_iterator VEC_I [V16QI V8HI V4SI V2DI]) > > +;; 128-bit int modes > +(define_mode_iterator VEC_I128 [V1TI TI]) > + > ;; Vector int modes for parity > (define_mode_iterator VEC_IP [V8HI > V4SI > @@ -1635,17 +1638,17 @@ > "") > > ;; No immediate version of this 128-bit instruction > -(define_expand "vashlv1ti3" > - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") > - (ashift:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") > - (match_operand:V1TI 2 "vsx_register_operand" "v")))] > +(define_expand "vashl<mode>3" > + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") > + (ashift:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand") > + (match_operand:VEC_I128 2 "vsx_register_operand")))] > "TARGET_TI_VECTOR_OPS" > { > /* Shift amount in needs to be put in bits[57:63] of 128-bit operand2. */ > - rtx tmp = gen_reg_rtx (V1TImode); > + rtx tmp = gen_reg_rtx (<MODE>mode); > > emit_insn(gen_xxswapd_v1ti (tmp, operands[2])); > - emit_insn(gen_altivec_vslq (operands[0], operands[1], tmp)); > + emit_insn(gen_altivec_vslq_<mode> (operands[0], operands[1], tmp)); > DONE; > }) > > @@ -1658,17 +1661,17 @@ > "") > > ;; No immediate version of this 128-bit instruction > -(define_expand "vlshrv1ti3" > - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") > - (lshiftrt:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") > - (match_operand:V1TI 2 "vsx_register_operand" "v")))] > +(define_expand "vlshr<mode>3" > + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") > + (lshiftrt:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand") > + (match_operand:VEC_I128 2 "vsx_register_operand")))] > "TARGET_TI_VECTOR_OPS" > { > /* Shift amount in needs to be put into bits[57:63] of 128-bit operand2. */ > - rtx tmp = gen_reg_rtx (V1TImode); > + rtx tmp = gen_reg_rtx (<MODE>mode); > > emit_insn(gen_xxswapd_v1ti (tmp, operands[2])); > - emit_insn(gen_altivec_vsrq (operands[0], operands[1], tmp)); > + emit_insn(gen_altivec_vsrq_<mode> (operands[0], operands[1], tmp)); > DONE; > }) > > diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md > index 998af3908ad..5be535808b3 100644 > --- a/gcc/config/rs6000/vsx.md > +++ b/gcc/config/rs6000/vsx.md > @@ -367,7 +367,7 @@ > UNSPEC_INSERTR > UNSPEC_REPLACE_ELT > UNSPEC_REPLACE_UN > - UNSPEC_XXSWAPD_V1TI > + UNSPEC_XXSWAPD_VEC_I128 > ]) double-check whitespace indentation. > > (define_int_iterator XVCVBF16 [UNSPEC_VSX_XVCVSPBF16 > @@ -3144,12 +3144,12 @@ > "xxpermdi %x0,%x1,%x1,2" > [(set_attr "type" "vecperm")]) > > -;; Swap upper/lower 64-bit values in a 128-bit vector > -(define_insn "xxswapd_v1ti" > - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") > - (unspec:V1TI [(match_operand:V1TI 1 "vsx_register_operand" "v") > - (parallel [(const_int 0)(const_int 1)])] > - UNSPEC_XXSWAPD_V1TI))] > +;; Swap upper/lower 64-bit values in V1TI or TI type > +(define_insn "xxswapd_<mode>" > + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") > + (unspec:VEC_I128 [(match_operand:VEC_I128 1 "vsx_register_operand" "v") > + (parallel [(const_int 0)(const_int 1)])] > + UNSPEC_XXSWAPD_VEC_I128))] > "TARGET_POWER10" > ;; AIX does not support extended mnemonic xxswapd. Use the basic > ;; mnemonic xxpermdi instead. > diff --git a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c > index d1e69cea021..b074d83bd68 100644 > --- a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c > +++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c > @@ -53,6 +53,18 @@ void print_i128(__int128_t val) > > void abort (void); > > +__attribute__((noinline)) > +__int128_t shift_right (__int128_t a, __uint128_t b) > +{ > + return a >> b; > +} > + > +__attribute__((noinline)) > +__int128_t shift_left (__int128_t a, __uint128_t b) > +{ > + return a << b; > +} > + > int main () > { > int i, result_int; > @@ -141,10 +153,12 @@ int main () > #endif > } > > - arg1 = 3; > + // arg1 = 3; > + arg1 = vec_result[0]; > uarg2 = 4; > expected_result = arg1*16; Just drop the lines that are commented out with "//" Here and below. > > + // result = shift_left(arg1, uarg2); > result = arg1 << uarg2; > > if (result != expected_result) { > @@ -225,10 +239,16 @@ int main () > #endif > } > > - arg1 = 48; > + // arg1 = 48; > + > + // use the previous result to try and keep gcc from doing the shift > + // at compile time > + arg1 = vec_uresult[0]; > uarg2 = 4; > expected_result = arg1/16; > > + //Not getting 128-bit shift inst generated > + // result = shift_right (arg1, uarg2); > result = arg1 >> uarg2; > > if (result != expected_result) {
Hi! On Tue, Aug 11, 2020 at 12:23:05PM -0700, Carl Love wrote: > +;; 128-bit int modes > +(define_mode_iterator VEC_I128 [V1TI TI]) We already have VSX_TI for this (in vsx.md). Rename that to something without VSX, and move it to vector.md or such? Maybe name it VEC_TI or anyTI. Do that renaming as a separate patch before this one? It is logically separate, and it is boring stuff, so putting it in a separate patch makes the non-boring stuff stand out more. (It would be better if we could just get rid of V1TI, but that isn't going to happen soon). > --- a/gcc/config/rs6000/vsx.md > +++ b/gcc/config/rs6000/vsx.md > @@ -367,7 +367,7 @@ > UNSPEC_INSERTR > UNSPEC_REPLACE_ELT > UNSPEC_REPLACE_UN > - UNSPEC_XXSWAPD_V1TI > + UNSPEC_XXSWAPD_VEC_I128 Why not just UNSPEC_XXSWAPD? And, why an unspec at all? Segher
Segher: On Thu, 2020-08-20 at 16:50 -0500, Segher Boessenkool wrote: > Hi! > > On Tue, Aug 11, 2020 at 12:23:05PM -0700, Carl Love wrote: > > +;; 128-bit int modes > > +(define_mode_iterator VEC_I128 [V1TI TI]) > > We already have VSX_TI for this (in vsx.md). Rename that to > something > without VSX, and move it to vector.md or such? Maybe name it VEC_TI > or anyTI. > > Do that renaming as a separate patch before this one? It is > logically > separate, and it is boring stuff, so putting it in a separate patch > makes the non-boring stuff stand out more. > > (It would be better if we could just get rid of V1TI, but that isn't > going to happen soon). > > > --- a/gcc/config/rs6000/vsx.md > > +++ b/gcc/config/rs6000/vsx.md > > @@ -367,7 +367,7 @@ > > UNSPEC_INSERTR > > UNSPEC_REPLACE_ELT > > UNSPEC_REPLACE_UN > > - UNSPEC_XXSWAPD_V1TI > > + UNSPEC_XXSWAPD_VEC_I128 > > Why not just UNSPEC_XXSWAPD? And, why an unspec at all? I am trying to figure out how to specify this without using an unpsec per your last comment. I changed the definition to: ;; Swap upper/lower 64-bit values in V1TI or TI type (define_insn "xxswapd_<mode>" [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") (vec_select:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand" "v") (parallel [(const_int 0)])))] "TARGET_POWER10" ;; AIX does not support extended mnemonic xxswapd. Use the basic ;; mnemonic xxpermdi instead. "xxpermdi %x0,%x1,%x1,2" [(set_attr "type" "vecperm")]) All of the swap definitions that I can see are based on using vec_select which seems to be the issue here. Not seeing anyway to do this without using unspec. Any thoughts? Carl
Hi! On Wed, Aug 26, 2020 at 01:27:44PM -0700, Carl Love wrote: > > > @@ -367,7 +367,7 @@ > > > UNSPEC_INSERTR > > > UNSPEC_REPLACE_ELT > > > UNSPEC_REPLACE_UN > > > - UNSPEC_XXSWAPD_V1TI > > > + UNSPEC_XXSWAPD_VEC_I128 > > > > Why not just UNSPEC_XXSWAPD? And, why an unspec at all? > > I am trying to figure out how to specify this without using an unpsec > per your last comment. I changed the definition to: > > ;; Swap upper/lower 64-bit values in V1TI or TI type > (define_insn "xxswapd_<mode>" > [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") > (vec_select:VEC_I128 > (match_operand:VEC_I128 1 "vsx_register_operand" "v") > (parallel [(const_int 0)])))] > "TARGET_POWER10" > ;; AIX does not support extended mnemonic xxswapd. Use the basic > ;; mnemonic xxpermdi instead. > "xxpermdi %x0,%x1,%x1,2" > [(set_attr "type" "vecperm")]) (define_insn "xxswapd_<mode>" [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") (subreg:VEC_I128 (vec_select:V2DI (match_operand:V2DI 1 "vsx_register_operand" "v") (parallel [(const_int 1) (const_int 0)])) 0))] or similar (i.e., just cast it to the type you want -- in hardware, all vectors are just an opaque 128 bits, but in RTL they have a type). (You probably want to cast operands[1] as well). Segher
diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md index 2763d920828..cba39852070 100644 --- a/gcc/config/rs6000/altivec.md +++ b/gcc/config/rs6000/altivec.md @@ -2219,10 +2219,10 @@ "vsl<VI_char> %0,%1,%2" [(set_attr "type" "vecsimple")]) -(define_insn "altivec_vslq" - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") - (ashift:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") - (match_operand:V1TI 2 "vsx_register_operand" "v")))] +(define_insn "altivec_vslq_<mode>" + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") + (ashift:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand" "v") + (match_operand:VEC_I128 2 "vsx_register_operand" "v")))] "TARGET_TI_VECTOR_OPS" /* Shift amount in needs to be in bits[57:63] of 128-bit operand. */ "vslq %0,%1,%2" @@ -2236,10 +2236,10 @@ "vsr<VI_char> %0,%1,%2" [(set_attr "type" "vecsimple")]) -(define_insn "altivec_vsrq" - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") - (lshiftrt:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") - (match_operand:V1TI 2 "vsx_register_operand" "v")))] +(define_insn "altivec_vsrq_<mode>" + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") + (lshiftrt:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand" "v") + (match_operand:VEC_I128 2 "vsx_register_operand" "v")))] "TARGET_TI_VECTOR_OPS" /* Shift amount in needs to be in bits[57:63] of 128-bit operand. */ "vsrq %0,%1,%2" diff --git a/gcc/config/rs6000/vector.md b/gcc/config/rs6000/vector.md index 2deff282076..682aabc4657 100644 --- a/gcc/config/rs6000/vector.md +++ b/gcc/config/rs6000/vector.md @@ -26,6 +26,9 @@ ;; Vector int modes (define_mode_iterator VEC_I [V16QI V8HI V4SI V2DI]) +;; 128-bit int modes +(define_mode_iterator VEC_I128 [V1TI TI]) + ;; Vector int modes for parity (define_mode_iterator VEC_IP [V8HI V4SI @@ -1635,17 +1638,17 @@ "") ;; No immediate version of this 128-bit instruction -(define_expand "vashlv1ti3" - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") - (ashift:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") - (match_operand:V1TI 2 "vsx_register_operand" "v")))] +(define_expand "vashl<mode>3" + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") + (ashift:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand") + (match_operand:VEC_I128 2 "vsx_register_operand")))] "TARGET_TI_VECTOR_OPS" { /* Shift amount in needs to be put in bits[57:63] of 128-bit operand2. */ - rtx tmp = gen_reg_rtx (V1TImode); + rtx tmp = gen_reg_rtx (<MODE>mode); emit_insn(gen_xxswapd_v1ti (tmp, operands[2])); - emit_insn(gen_altivec_vslq (operands[0], operands[1], tmp)); + emit_insn(gen_altivec_vslq_<mode> (operands[0], operands[1], tmp)); DONE; }) @@ -1658,17 +1661,17 @@ "") ;; No immediate version of this 128-bit instruction -(define_expand "vlshrv1ti3" - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") - (lshiftrt:V1TI (match_operand:V1TI 1 "vsx_register_operand" "v") - (match_operand:V1TI 2 "vsx_register_operand" "v")))] +(define_expand "vlshr<mode>3" + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") + (lshiftrt:VEC_I128 (match_operand:VEC_I128 1 "vsx_register_operand") + (match_operand:VEC_I128 2 "vsx_register_operand")))] "TARGET_TI_VECTOR_OPS" { /* Shift amount in needs to be put into bits[57:63] of 128-bit operand2. */ - rtx tmp = gen_reg_rtx (V1TImode); + rtx tmp = gen_reg_rtx (<MODE>mode); emit_insn(gen_xxswapd_v1ti (tmp, operands[2])); - emit_insn(gen_altivec_vsrq (operands[0], operands[1], tmp)); + emit_insn(gen_altivec_vsrq_<mode> (operands[0], operands[1], tmp)); DONE; }) diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md index 998af3908ad..5be535808b3 100644 --- a/gcc/config/rs6000/vsx.md +++ b/gcc/config/rs6000/vsx.md @@ -367,7 +367,7 @@ UNSPEC_INSERTR UNSPEC_REPLACE_ELT UNSPEC_REPLACE_UN - UNSPEC_XXSWAPD_V1TI + UNSPEC_XXSWAPD_VEC_I128 ]) (define_int_iterator XVCVBF16 [UNSPEC_VSX_XVCVSPBF16 @@ -3144,12 +3144,12 @@ "xxpermdi %x0,%x1,%x1,2" [(set_attr "type" "vecperm")]) -;; Swap upper/lower 64-bit values in a 128-bit vector -(define_insn "xxswapd_v1ti" - [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") - (unspec:V1TI [(match_operand:V1TI 1 "vsx_register_operand" "v") - (parallel [(const_int 0)(const_int 1)])] - UNSPEC_XXSWAPD_V1TI))] +;; Swap upper/lower 64-bit values in V1TI or TI type +(define_insn "xxswapd_<mode>" + [(set (match_operand:VEC_I128 0 "vsx_register_operand" "=v") + (unspec:VEC_I128 [(match_operand:VEC_I128 1 "vsx_register_operand" "v") + (parallel [(const_int 0)(const_int 1)])] + UNSPEC_XXSWAPD_VEC_I128))] "TARGET_POWER10" ;; AIX does not support extended mnemonic xxswapd. Use the basic ;; mnemonic xxpermdi instead. diff --git a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c index d1e69cea021..b074d83bd68 100644 --- a/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c +++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c @@ -53,6 +53,18 @@ void print_i128(__int128_t val) void abort (void); +__attribute__((noinline)) +__int128_t shift_right (__int128_t a, __uint128_t b) +{ + return a >> b; +} + +__attribute__((noinline)) +__int128_t shift_left (__int128_t a, __uint128_t b) +{ + return a << b; +} + int main () { int i, result_int; @@ -141,10 +153,12 @@ int main () #endif } - arg1 = 3; + // arg1 = 3; + arg1 = vec_result[0]; uarg2 = 4; expected_result = arg1*16; + // result = shift_left(arg1, uarg2); result = arg1 << uarg2; if (result != expected_result) { @@ -225,10 +239,16 @@ int main () #endif } - arg1 = 48; + // arg1 = 48; + + // use the previous result to try and keep gcc from doing the shift + // at compile time + arg1 = vec_uresult[0]; uarg2 = 4; expected_result = arg1/16; + //Not getting 128-bit shift inst generated + // result = shift_right (arg1, uarg2); result = arg1 >> uarg2; if (result != expected_result) {
Segher, Will: Patch 4 adds 128-bit integer shift instruction support. Carl Love --------------------------------------------------------- Test 128-bit shifts for just the int128 type. gcc/ChangeLog 2020-08-10 Carl Love <cel@us.ibm.com> * config/rs6000/altivec.md (altivec_vslq, altivec_vsrq): Add mode VEC_I128. * config/rs6000/vector.md (VEC_I128): New mode iterator. (vashlv1ti3): Change to vashl<mode>3, mode VEC_I128. (vlshrv1ti3): Change to vlshr<mode>3, mode VEC_I128. * config/rs6000/vsx.md (UNSPEC_XXSWAPD_V1TI): Change to UNSPEC_XXSWAPD_VEC_I128. (xxswapd_v1ti): Change to xxswapd_<mode>, mode VEC_I128. gcc/testsuite/ChangeLog 2020-08-10 Carl Love <cel@us.ibm.com> gcc.target/powerpc/int_128bit-runnable.c: Add shift_right, shift_left tests. --- gcc/config/rs6000/altivec.md | 16 +++++------ gcc/config/rs6000/vector.md | 27 ++++++++++--------- gcc/config/rs6000/vsx.md | 14 +++++----- .../gcc.target/powerpc/int_128bit-runnable.c | 24 +++++++++++++++-- 4 files changed, 52 insertions(+), 29 deletions(-)