public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
To: Christophe Lyon <Christophe.Lyon@arm.com>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH] arm: Fix constant immediates predicates and constraints for some MVE builtins
Date: Thu, 8 Sep 2022 13:57:54 +0000	[thread overview]
Message-ID: <PAXPR08MB69260B77CB5A385864FB5F6F93409@PAXPR08MB6926.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <7759712b-8f81-5668-4fee-14e89bad5177@arm.com>



> -----Original Message-----
> From: Christophe Lyon <Christophe.Lyon@arm.com>
> Sent: Wednesday, September 7, 2022 5:59 PM
> To: Kyrylo Tkachov <Kyrylo.Tkachov@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.Lyon@arm.com>
> >> Sent: Wednesday, September 7, 2022 2:41 PM
> >> To: Kyrylo Tkachov <Kyrylo.Tkachov@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.com@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.lyon@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
> >>>

      reply	other threads:[~2022-09-08 13:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-07 13:02 Christophe Lyon
2022-09-07 13:34 ` Kyrylo Tkachov
2022-09-07 13:41   ` Christophe Lyon
2022-09-07 13:42     ` Kyrylo Tkachov
2022-09-07 16:58       ` Christophe Lyon
2022-09-08 13:57         ` Kyrylo Tkachov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PAXPR08MB69260B77CB5A385864FB5F6F93409@PAXPR08MB6926.eurprd08.prod.outlook.com \
    --to=kyrylo.tkachov@arm.com \
    --cc=Christophe.Lyon@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).