> -----Original Message----- > From: Christophe Lyon <christophe.l...@arm.com> > Sent: Wednesday, September 7, 2022 5:59 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:42, Kyrylo Tkachov wrote: > > > > > >> -----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. > > OK, just ran a bootstrap on arm-linux-gnueabihf, no issue, and > regression tested on a cross arm-eabi with the default > RUNTESTFLAGS/target-board, no issue either. >
Ok, thanks for testing. Kyrill > Thanks, > > Christophe > > > 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 > >>>
