| Message ID | 58C11D19.30600@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 09, 2017 at 09:15:05AM +0000, Kyrill Tkachov wrote: > Hi all, > > This patch fixes the vec_select errors found by Jakub's genrecog validation improvements: > ../../gcc/config/aarch64/aarch64-simd.md:79:1: DImode of first vec_select operand is not a vector mode > ../../gcc/config/aarch64/aarch64-simd.md:79:1: DFmode of first vec_select operand is not a vector mode > ../../gcc/config/aarch64/aarch64-simd.md:588:1: DImode of first vec_select operand is not a vector mode > ../../gcc/config/aarch64/aarch64-simd.md:588:1: DFmode of first vec_select operand is not a vector mode > ../../gcc/config/aarch64/aarch64-simd.md:3192:1: DFmode of first vec_select operand is not a vector mode > > The first four are in the DUP and INS lane patterns. They iterate over the > VALL_F16 or VALL mode iterators that include V2DI and V2DF. The VSWAP_WIDTH > attribute for these modes are the scalar DI and DF, which are not > valid as modes of a vec_select operand. > > The problematic patterns deal with lane copies between elements of 64-bit > vectors to 128-bit vectors and vice versa. 64 to 64-bit and 128 to 128-bit > copies are handled in separate patterns (aarch64_dup_lane<mode> and > *aarch64_simd_vec_copy_lane<mode>). > > But there is no variant of those instructions to copy a 64-bit element from a > 128-bit vector to a 64-bit vector (A DUP V<n>.1D, V<m>.D[0] for example). > > Similarly, there is no INS instruction to move from a 128-bit vector of 2 > 64-bit elements to a 64-bit "vector" of one 64-bit element. James also > noticed that these patterns don't handle HFmode, so this patch also adds that > to their iterator. > > The last error is the *aarch64_vgetfmulx<mode> pattern. It uses the VDQF_DF > iterator on the vec_select argument. But VDQF_DF includes DFmode, which is > scalar and thus not valid here. > > I think the best course of action here is to not iterate over DFmode in this > pattern. It's not clear what a vec_select was intended to express for that > case and there are other patterns that do a fmulx on scalar 64-bit operands. > Removing the DFmode from the iterated > modes fixes the error and the relevant intrinsics tests run just fine (plus > some other more rigorous AdvSIMD intrinsics tests that I run). > > With these fixes the genrecog validation doesn't complain for aarch64. > Bootstrapped and tested on aarch64-none-linux-gnu. > > Ok for trunk? OK. Thanks, James > 2017-03-09 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/79913 > * config/aarch64/iterators.md (VALL_F16_NO_V2Q): New mode iterator. > (VALL_NO_V2Q): Likewise. > (VDQF_DF): Delete. > * config/aarch64/aarch64-simd.md > (aarch64_dup_lane_<vswap_width_name><mode>): Use VALL_F16_NO_V2Q > iterator. > (*aarch64_simd_vec_copy_lane_<vswap_width_name><mode>): Use > VALL_NO_V2Q mode iterator. > (*aarch64_vgetfmulx<mode>): Use VDQF iterator.
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md index b61f79a..7ad3a76 100644 --- a/gcc/config/aarch64/aarch64-simd.md +++ b/gcc/config/aarch64/aarch64-simd.md @@ -77,8 +77,8 @@ (define_insn "aarch64_dup_lane<mode>" ) (define_insn "aarch64_dup_lane_<vswap_width_name><mode>" - [(set (match_operand:VALL_F16 0 "register_operand" "=w") - (vec_duplicate:VALL_F16 + [(set (match_operand:VALL_F16_NO_V2Q 0 "register_operand" "=w") + (vec_duplicate:VALL_F16_NO_V2Q (vec_select:<VEL> (match_operand:<VSWAP_WIDTH> 1 "register_operand" "w") (parallel [(match_operand:SI 2 "immediate_operand" "i")]) @@ -586,14 +586,14 @@ (define_insn "*aarch64_simd_vec_copy_lane<mode>" ) (define_insn "*aarch64_simd_vec_copy_lane_<vswap_width_name><mode>" - [(set (match_operand:VALL 0 "register_operand" "=w") - (vec_merge:VALL - (vec_duplicate:VALL + [(set (match_operand:VALL_F16_NO_V2Q 0 "register_operand" "=w") + (vec_merge:VALL_F16_NO_V2Q + (vec_duplicate:VALL_F16_NO_V2Q (vec_select:<VEL> (match_operand:<VSWAP_WIDTH> 3 "register_operand" "w") (parallel [(match_operand:SI 4 "immediate_operand" "i")]))) - (match_operand:VALL 1 "register_operand" "0") + (match_operand:VALL_F16_NO_V2Q 1 "register_operand" "0") (match_operand:SI 2 "immediate_operand" "i")))] "TARGET_SIMD" { @@ -3194,7 +3194,7 @@ (define_insn "*aarch64_vgetfmulx<mode>" (unspec:<VEL> [(match_operand:<VEL> 1 "register_operand" "w") (vec_select:<VEL> - (match_operand:VDQF_DF 2 "register_operand" "w") + (match_operand:VDQF 2 "register_operand" "w") (parallel [(match_operand:SI 3 "immediate_operand" "i")]))] UNSPEC_FMULX))] "TARGET_SIMD" diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md index c59d31e..1ddf6ad 100644 --- a/gcc/config/aarch64/iterators.md +++ b/gcc/config/aarch64/iterators.md @@ -101,7 +101,6 @@ (define_mode_iterator VHSDF [(V4HF "TARGET_SIMD_F16INST") V2SF V4SF V2DF]) ;; Vector Float modes, and DF. -(define_mode_iterator VDQF_DF [V2SF V4SF V2DF DF]) (define_mode_iterator VHSDF_DF [(V4HF "TARGET_SIMD_F16INST") (V8HF "TARGET_SIMD_F16INST") V2SF V4SF V2DF DF]) @@ -133,6 +132,10 @@ (define_mode_iterator VALL [V8QI V16QI V4HI V8HI V2SI V4SI V2DI V2SF V4SF V2DF]) (define_mode_iterator VALL_F16 [V8QI V16QI V4HI V8HI V2SI V4SI V2DI V4HF V8HF V2SF V4SF V2DF]) +;; The VALL_F16 modes except the 128-bit 2-element ones. +(define_mode_iterator VALL_F16_NO_V2Q [V8QI V16QI V4HI V8HI V2SI V4SI + V4HF V8HF V2SF V4SF]) + ;; All vector modes barring HF modes, plus DI. (define_mode_iterator VALLDI [V8QI V16QI V4HI V8HI V2SI V4SI V2DI V2SF V4SF V2DF DI])
Hi all, This patch fixes the vec_select errors found by Jakub's genrecog validation improvements: ../../gcc/config/aarch64/aarch64-simd.md:79:1: DImode of first vec_select operand is not a vector mode ../../gcc/config/aarch64/aarch64-simd.md:79:1: DFmode of first vec_select operand is not a vector mode ../../gcc/config/aarch64/aarch64-simd.md:588:1: DImode of first vec_select operand is not a vector mode ../../gcc/config/aarch64/aarch64-simd.md:588:1: DFmode of first vec_select operand is not a vector mode ../../gcc/config/aarch64/aarch64-simd.md:3192:1: DFmode of first vec_select operand is not a vector mode The first four are in the DUP and INS lane patterns. They iterate over the VALL_F16 or VALL mode iterators that include V2DI and V2DF. The VSWAP_WIDTH attribute for these modes are the scalar DI and DF, which are not valid as modes of a vec_select operand. The problematic patterns deal with lane copies between elements of 64-bit vectors to 128-bit vectors and vice versa. 64 to 64-bit and 128 to 128-bit copies are handled in separate patterns (aarch64_dup_lane<mode> and *aarch64_simd_vec_copy_lane<mode>). But there is no variant of those instructions to copy a 64-bit element from a 128-bit vector to a 64-bit vector (A DUP V<n>.1D, V<m>.D[0] for example). Similarly, there is no INS instruction to move from a 128-bit vector of 2 64-bit elements to a 64-bit "vector" of one 64-bit element. James also noticed that these patterns don't handle HFmode, so this patch also adds that to their iterator. The last error is the *aarch64_vgetfmulx<mode> pattern. It uses the VDQF_DF iterator on the vec_select argument. But VDQF_DF includes DFmode, which is scalar and thus not valid here. I think the best course of action here is to not iterate over DFmode in this pattern. It's not clear what a vec_select was intended to express for that case and there are other patterns that do a fmulx on scalar 64-bit operands. Removing the DFmode from the iterated modes fixes the error and the relevant intrinsics tests run just fine (plus some other more rigorous AdvSIMD intrinsics tests that I run). With these fixes the genrecog validation doesn't complain for aarch64. Bootstrapped and tested on aarch64-none-linux-gnu. Ok for trunk? Thanks, Kyrill 2017-03-09 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR target/79913 * config/aarch64/iterators.md (VALL_F16_NO_V2Q): New mode iterator. (VALL_NO_V2Q): Likewise. (VDQF_DF): Delete. * config/aarch64/aarch64-simd.md (aarch64_dup_lane_<vswap_width_name><mode>): Use VALL_F16_NO_V2Q iterator. (*aarch64_simd_vec_copy_lane_<vswap_width_name><mode>): Use VALL_NO_V2Q mode iterator. (*aarch64_vgetfmulx<mode>): Use VDQF iterator.