From: Christophe Lyon <christophe.lyon@arm.com>
To: Kyrylo Tkachov <Kyrylo.Tkachov@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: Wed, 7 Sep 2022 18:58:41 +0200 [thread overview]
Message-ID: <7759712b-8f81-5668-4fee-14e89bad5177@arm.com> (raw)
In-Reply-To: <AS8PR08MB6918355EFC28AD1EC83C6B9B93419@AS8PR08MB6918.eurprd08.prod.outlook.com>
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.
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
>>>
next prev parent reply other threads:[~2022-09-07 16: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 [this message]
2022-09-08 13:57 ` Kyrylo Tkachov
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=7759712b-8f81-5668-4fee-14e89bad5177@arm.com \
--to=christophe.lyon@arm.com \
--cc=Kyrylo.Tkachov@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).