From: Sudakshina Das <sudi.das@arm.com>
To: Christophe Lyon <christophe.lyon@linaro.org>
Cc: "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>,
Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,
nd <nd@arm.com>
Subject: Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts
Date: Tue, 27 Mar 2018 16:46:00 -0000 [thread overview]
Message-ID: <2297f1fe-a788-78e1-76f4-ae8806f6f7f4@arm.com> (raw)
In-Reply-To: <53ba93f9-e679-7bc2-eefc-28877c61ae7f@arm.com>
On 21/03/18 11:40, Sudakshina Das wrote:
> Hi
>
> On 21/03/18 08:51, Christophe Lyon wrote:
>> On 20 March 2018 at 11:58, Sudakshina Das <sudi.das@arm.com> wrote:
>>> Hi
>>>
>>> On 20/03/18 10:03, Richard Earnshaw (lists) wrote:
>>>>
>>>> On 14/03/18 10:11, Sudakshina Das wrote:
>>>>>
>>>>> Hi
>>>>>
>>>>> This patch fixes PR82989 so that we avoid NEON instructions when
>>>>> -mneon-for-64bits is not enabled. This is more of a short term fix for
>>>>> the real deeper problem of making and early decision of choosing or
>>>>> rejecting NEON instructions. There is now a new ticket PR84467 to deal
>>>>> with the longer term solution.
>>>>> (Please refer to the discussion in the bug report for more details).
>>>>>
>>>>> Testing: Bootstrapped and regtested on arm-none-linux-gnueabihf and
>>>>> added a new test case based on the test given on the bug report.
>>>>>
>>>>> Ok for trunk and backports for gcc-7 and gcc-6 branches?
>>>>>
>>>>
>>>> OK for trunk. Please leave it a couple of days before backporting to
>>>> ensure that the testcase doesn't tickle any multilib issues.
>>>>
>>>> R.
>>>
>>>
>>> Thanks. Committed to trunk as r258677. Will wait a week for backporting.
Backported both the commits of trunks to gcc-7 as r258883 and to gcc-6
as r258884 (Reg-tested for both)
Thanks
Sudi
>>>
>>> Sudi
>>>
>>
>> Hi Sudi,
>>
>> I've noticed that:
>> FAIL:Â Â Â gcc.target/arm/pr82989.c scan-assembler-times lsl\\tr[0-9]+,
>> r[0-9]+, r[0-9] 2
>> FAIL:Â Â Â gcc.target/arm/pr82989.c scan-assembler-times lsr\\tr[0-9]+,
>> r[0-9]+, r[0-9] 2
>> on target armeb-none-linux-gnueabihf
>> --with-mode thumb
>> --with-cpu cortex-a9
>> --with-fpu neon-fp16
>>
>> The tests pass when using --with-mode arm
>>
>> Can you check?
>
> Yes I see this as well. Sorry about this. I am testing a quick fix for
> this at the moment.
>
> Thanks
> Sudi
>
>>
>> Thanks
>>
>> Christophe
>>
>>>
>>>>
>>>>> Sudi
>>>>>
>>>>>
>>>>> *** gcc/ChangeLog ***
>>>>>
>>>>> 2018-03-14 Sudakshina Das <sudi.das@arm.com>
>>>>>
>>>>> Â Â Â Â Â * config/arm/neon.md (ashldi3_neon): Update ?s for constraints
>>>>> Â Â Â Â Â to favor GPR over NEON registers.
>>>>> Â Â Â Â Â (<shift>di3_neon): Likewise.
>>>>>
>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>
>>>>> 2018-03-14 Sudakshina Das <sudi.das@arm.com>
>>>>>
>>>>> Â Â Â Â Â * gcc.target/arm/pr82989.c: New test.
>>>>>
>>>>> pr82989.diff
>>>>>
>>>>>
>>>>> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
>>>>> index 6a6f5d7..1646b21 100644
>>>>> --- a/gcc/config/arm/neon.md
>>>>> +++ b/gcc/config/arm/neon.md
>>>>> @@ -1180,12 +1180,12 @@
>>>>> Â Â )
>>>>> Â Â Â Â (define_insn_and_split "ashldi3_neon"
>>>>> -Â [(set (match_operand:DI 0 "s_register_operand"Â Â Â Â Â Â Â Â Â Â "= w,
>>>>> w,?&r,?r,?&r, ?w,w")
>>>>> -Â Â Â Â Â Â (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w,
>>>>> w, 0r,
>>>>> 0, r, 0w,w")
>>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 2 "general_operand"Â Â Â "rUm,
>>>>> i, r,
>>>>> i, i,rUm,i")))
>>>>> -Â Â (clobber (match_scratch:SI 3
>>>>> "= X,
>>>>> X,?&r, X, X, X,X"))
>>>>> -Â Â (clobber (match_scratch:SI 4
>>>>> "= X,
>>>>> X,?&r, X, X, X,X"))
>>>>> -Â Â (clobber (match_scratch:DI 5
>>>>> "=&w,
>>>>> X, X, X, X, &w,X"))
>>>>> +Â [(set (match_operand:DI 0 "s_register_operand"Â Â Â Â Â Â Â Â Â Â "= w,
>>>>> w, &r,
>>>>> r, &r, ?w,?w")
>>>>> +Â Â Â Â Â Â (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w,
>>>>> w, 0r,
>>>>> 0, r, 0w, w")
>>>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 2 "general_operand"Â Â Â "rUm,
>>>>> i, r,
>>>>> i, i,rUm, i")))
>>>>> +Â Â (clobber (match_scratch:SI 3
>>>>> "= X,
>>>>> X, &r, X, X, X, X"))
>>>>> +Â Â (clobber (match_scratch:SI 4
>>>>> "= X,
>>>>> X, &r, X, X, X, X"))
>>>>> +Â Â (clobber (match_scratch:DI 5
>>>>> "=&w,
>>>>> X, X, X, X, &w, X"))
>>>>> Â Â Â Â Â (clobber (reg:CC_C CC_REGNUM))]
>>>>> Â Â Â Â "TARGET_NEON"
>>>>> Â Â Â Â "#"
>>>>> @@ -1276,7 +1276,7 @@
>>>>> Â Â ;; ashrdi3_neon
>>>>> Â Â ;; lshrdi3_neon
>>>>> Â Â (define_insn_and_split "<shift>di3_neon"
>>>>> -Â [(set (match_operand:DI 0 "s_register_operand"Â Â Â Â Â Â Â Â Â Â Â "= w,
>>>>> w,?&r,?r,?&r,?w,?w")
>>>>> +Â [(set (match_operand:DI 0 "s_register_operand"Â Â Â Â Â Â Â Â Â Â Â "= w,
>>>>> w, &r,
>>>>> r, &r,?w,?w")
>>>>> Â Â Â Â Â Â Â Â (RSHIFTS:DI (match_operand:DI 1 "s_register_operand" " 0w,
>>>>> w, 0r,
>>>>> 0, r,0w, w")
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 2 "reg_or_int_operand" "Â r,
>>>>> i, r,
>>>>> i, i, r, i")))
>>>>> Â Â Â Â Â (clobber (match_scratch:SI 3
>>>>> "=2r, X, &r, X, X,2r, X"))
>>>>> diff --git a/gcc/testsuite/gcc.target/arm/pr82989.c
>>>>> b/gcc/testsuite/gcc.target/arm/pr82989.c
>>>>> new file mode 100644
>>>>> index 0000000..1295ee6
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/gcc.target/arm/pr82989.c
>>>>> @@ -0,0 +1,38 @@
>>>>> +/* PR target/82989 */
>>>>> +/* { dg-do compile } */
>>>>> +/* { dg-require-effective-target arm_neon_ok } */
>>>>> +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
>>>>> "-mcpu=*" } { "-mcpu=cortex-a8" } } */
>>>>> +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
>>>>> "-mfpu=*" } { "-mfpu=neon" } } */
>>>>> +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
>>>>> "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
>>>>> +/* { dg-options "-O2 -mcpu=cortex-a8 -mfpu=neon -mfloat-abi=hard"
>>>>> } */
>>>>> +/* { dg-add-options arm_neon } */
>>>>> +
>>>>> +typedef unsigned long long uint64_t;
>>>>> +
>>>>> +void f_shr_imm (uint64_t *a )
>>>>> +{
>>>>> +Â *a += *a >> 32;
>>>>> +}
>>>>> +/* { dg-final { scan-assembler-not "vshr*" } } */
>>>>> +
>>>>> +void f_shr_reg (uint64_t *a, uint64_t b)
>>>>> +{
>>>>> +Â *a += *a >> b;
>>>>> +}
>>>>> +/* { dg-final { scan-assembler-not "vshl*" } } */
>>>>> +/* Only 2 times for f_shr_reg. f_shr_imm should not have any. */
>>>>> +/* { dg-final { scan-assembler-times {lsr\tr[0-9]+, r[0-9]+,
>>>>> r[0-9]} 2 }
>>>>> } */
>>>>> +
>>>>> +void f_shl_imm (uint64_t *a)
>>>>> +{
>>>>> +Â *a += *a << 32;
>>>>> +}
>>>>> +/* { dg-final { scan-assembler-not "vshl*" } } */
>>>>> +
>>>>> +void f_shl_reg (uint64_t *a, uint64_t b)
>>>>> +{
>>>>> +Â *a += *a << b;
>>>>> +}
>>>>> +/* { dg-final { scan-assembler-not "vshl*" } } */
>>>>> +/* Only 2 times for f_shl_reg. f_shl_imm should not have any. */
>>>>> +/* { dg-final { scan-assembler-times {lsl\tr[0-9]+, r[0-9]+,
>>>>> r[0-9]} 2 }
>>>>> } */
>>>>>
>>>>
>>>
>
prev parent reply other threads:[~2018-03-27 15:26 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-14 10:55 Sudakshina Das
2018-03-20 10:08 ` Richard Earnshaw (lists)
2018-03-20 11:05 ` Sudakshina Das
2018-03-21 8:59 ` Christophe Lyon
2018-03-21 12:01 ` Sudakshina Das
2018-03-27 16:46 ` Sudakshina Das [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=2297f1fe-a788-78e1-76f4-ae8806f6f7f4@arm.com \
--to=sudi.das@arm.com \
--cc=Ramana.Radhakrishnan@arm.com \
--cc=Richard.Earnshaw@arm.com \
--cc=christophe.lyon@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=kyrylo.tkachov@foss.arm.com \
--cc=nd@arm.com \
/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).