> -----Original Message----- > From: Christophe Lyon <christophe.l...@arm.com> > Sent: Wednesday, September 7, 2022 2:41 PM > To: Kyrylo Tkachov <kyrylo.tkac...@arm.com>; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] arm: Fix constant immediates predicates and > constraints for some MVE builtins > > > > On 9/7/22 15:34, Kyrylo Tkachov wrote: > > Hi Christophe, > > > >> -----Original Message----- > >> From: Gcc-patches <gcc-patches- > >> bounces+kyrylo.tkachov=arm....@gcc.gnu.org> On Behalf Of Christophe > >> Lyon via Gcc-patches > >> Sent: Wednesday, September 7, 2022 2:03 PM > >> To: gcc-patches@gcc.gnu.org > >> Subject: [PATCH] arm: Fix constant immediates predicates and constraints > for > >> some MVE builtins > >> > >> Several MVE builtins incorrectly use the same predicate/constraint > >> pair for several modes, which does not match the specification. > >> This patch uses the appropriate iterator instead. > >> > > > > This looks ok to me. > > I presume you've tested this appropriately? > > I tested it manually with an offending testcase. > > Unfortunately, the existing testcases all use '1' as immediate, so this > does not really check the boundaries. We do plan to improve the existing > tests in a later patch that will more generally improve the MVE tests.
Sure, improving the tests is definitely worth it here. I meant more in the context of a standard bootstrap and testsuite run. Thanks, Kyrill > > Christophe > > > If so, ok for trunk. > > Thanks, > > Kyrill > > > >> 2022-09-06 Christophe Lyon <christophe.l...@arm.com> > >> > >> gcc/ > >> * config/arm/mve.md (mve_vqshluq_n_s<mode>): Use > >> MVE_pred/MVE_constraint instead of mve_imm_7/Ra. > >> (mve_vqshluq_m_n_s<mode>): Likewise. > >> (mve_vqrshrnbq_n_<supf><mode>): Use > >> MVE_pred3/MVE_constraint3 > >> instead of mve_imm_8/Rb. > >> (mve_vqrshrunbq_n_s<mode>): Likewise. > >> (mve_vqrshrntq_n_<supf><mode>): Likewise. > >> (mve_vqrshruntq_n_s<mode>): Likewise. > >> (mve_vrshrnbq_n_<supf><mode>): Likewise. > >> (mve_vrshrntq_n_<supf><mode>): Likewise. > >> (mve_vqrshrnbq_m_n_<supf><mode>): Likewise. > >> (mve_vqrshrntq_m_n_<supf><mode>): Likewise. > >> (mve_vrshrnbq_m_n_<supf><mode>): Likewise. > >> (mve_vrshrntq_m_n_<supf><mode>): Likewise. > >> (mve_vqrshrunbq_m_n_s<mode>): Likewise. > >> (mve_vsriq_n_<supf><mode): Use MVE_pred2/MVE_constraint2 > >> instead > >> of mve_imm_selective_upto_8/Rg. > >> (mve_vsriq_m_n_<supf><mode>): Likewise. > >> --- > >> gcc/config/arm/mve.md | 30 +++++++++++++++--------------- > >> 1 file changed, 15 insertions(+), 15 deletions(-) > >> > >> diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md > >> index c4dec01baac..714178609f7 100644 > >> --- a/gcc/config/arm/mve.md > >> +++ b/gcc/config/arm/mve.md > >> @@ -1624,7 +1624,7 @@ (define_insn "mve_vqshluq_n_s<mode>" > >> [ > >> (set (match_operand:MVE_2 0 "s_register_operand" "=w") > >> (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand" > >> "w") > >> - (match_operand:SI 2 "mve_imm_7" "Ra")] > >> + (match_operand:SI 2 "<MVE_pred>" > >> "<MVE_constraint>")] > >> VQSHLUQ_N_S)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -2615,7 +2615,7 @@ (define_insn > "mve_vqrshrnbq_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 > >> "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb")] > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>")] > >> VQRSHRNBQ_N)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -2630,7 +2630,7 @@ (define_insn "mve_vqrshrunbq_n_s<mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 > >> "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb")] > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>")] > >> VQRSHRUNBQ_N_S)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -3570,7 +3570,7 @@ (define_insn "mve_vsriq_n_<supf><mode>" > >> (set (match_operand:MVE_2 0 "s_register_operand" "=w") > >> (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand" > >> "0") > >> (match_operand:MVE_2 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_selective_upto_8" > >> "Rg")] > >> + (match_operand:SI 3 "<MVE_pred2>" > >> "<MVE_constraint2>")] > >> VSRIQ_N)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -4473,7 +4473,7 @@ (define_insn > "mve_vqrshrntq_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb")] > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>")] > >> VQRSHRNTQ_N)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -4489,7 +4489,7 @@ (define_insn "mve_vqrshruntq_n_s<mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb")] > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>")] > >> VQRSHRUNTQ_N_S)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -4777,7 +4777,7 @@ (define_insn > "mve_vrshrnbq_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb")] > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>")] > >> VRSHRNBQ_N)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -4793,7 +4793,7 @@ (define_insn "mve_vrshrntq_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb")] > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>")] > >> VRSHRNTQ_N)) > >> ] > >> "TARGET_HAVE_MVE" > >> @@ -4987,7 +4987,7 @@ (define_insn "mve_vqshluq_m_n_s<mode>" > >> (set (match_operand:MVE_2 0 "s_register_operand" "=w") > >> (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand" > >> "0") > >> (match_operand:MVE_2 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_7" "Ra") > >> + (match_operand:SI 3 "<MVE_pred>" > >> "<MVE_constraint>") > >> (match_operand:<MVE_VPRED> 4 > >> "vpr_register_operand" "Up")] > >> VQSHLUQ_M_N_S)) > >> ] > >> @@ -5019,7 +5019,7 @@ (define_insn "mve_vsriq_m_n_<supf><mode>" > >> (set (match_operand:MVE_2 0 "s_register_operand" "=w") > >> (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand" > >> "0") > >> (match_operand:MVE_2 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_selective_upto_8" "Rg") > >> + (match_operand:SI 3 "<MVE_pred2>" > >> "<MVE_constraint2>") > >> (match_operand:<MVE_VPRED> 4 > >> "vpr_register_operand" "Up")] > >> VSRIQ_M_N)) > >> ] > >> @@ -6138,7 +6138,7 @@ (define_insn > >> "mve_vqrshrnbq_m_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb") > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>") > >> (match_operand:<MVE_VPRED> 4 > >> "vpr_register_operand" "Up")] > >> VQRSHRNBQ_M_N)) > >> ] > >> @@ -6155,7 +6155,7 @@ (define_insn > >> "mve_vqrshrntq_m_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb") > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>") > >> (match_operand:<MVE_VPRED> 4 > >> "vpr_register_operand" "Up")] > >> VQRSHRNTQ_M_N)) > >> ] > >> @@ -6223,7 +6223,7 @@ (define_insn > >> "mve_vrshrnbq_m_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb") > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>") > >> (match_operand:<MVE_VPRED> 4 > >> "vpr_register_operand" "Up")] > >> VRSHRNBQ_M_N)) > >> ] > >> @@ -6240,7 +6240,7 @@ (define_insn > "mve_vrshrntq_m_n_<supf><mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb") > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>") > >> (match_operand:<MVE_VPRED> 4 > >> "vpr_register_operand" "Up")] > >> VRSHRNTQ_M_N)) > >> ] > >> @@ -6461,7 +6461,7 @@ (define_insn > "mve_vqrshrunbq_m_n_s<mode>" > >> (set (match_operand:<V_narrow_pack> 0 "s_register_operand" "=w") > >> (unspec:<V_narrow_pack> [(match_operand:<V_narrow_pack> 1 > >> "s_register_operand" "0") > >> (match_operand:MVE_5 2 "s_register_operand" "w") > >> - (match_operand:SI 3 "mve_imm_8" "Rb") > >> + (match_operand:SI 3 "<MVE_pred3>" > >> "<MVE_constraint3>") > >> (match_operand:<MVE_VPRED> 4 > >> "vpr_register_operand" "Up")] > >> VQRSHRUNBQ_M_N_S)) > >> ] > >> -- > >> 2.34.1 > >
