* [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts
@ 2018-03-14 10:55 Sudakshina Das
2018-03-20 10:08 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 6+ messages in thread
From: Sudakshina Das @ 2018-03-14 10:55 UTC (permalink / raw)
To: gcc-patches; +Cc: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
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?
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.
[-- Attachment #2: pr82989.diff --]
[-- Type: text/x-patch, Size: 3189 bytes --]
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 } } */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts
2018-03-14 10:55 [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts Sudakshina Das
@ 2018-03-20 10:08 ` Richard Earnshaw (lists)
2018-03-20 11:05 ` Sudakshina Das
0 siblings, 1 reply; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2018-03-20 10:08 UTC (permalink / raw)
To: Sudakshina Das, gcc-patches; +Cc: Kyrill Tkachov, Ramana Radhakrishnan, nd
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.
> 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 } } */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts
2018-03-20 10:08 ` Richard Earnshaw (lists)
@ 2018-03-20 11:05 ` Sudakshina Das
2018-03-21 8:59 ` Christophe Lyon
0 siblings, 1 reply; 6+ messages in thread
From: Sudakshina Das @ 2018-03-20 11:05 UTC (permalink / raw)
To: Richard Earnshaw (lists), gcc-patches
Cc: Kyrill Tkachov, Ramana Radhakrishnan, nd
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.
Sudi
>
>> 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 } } */
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts
2018-03-20 11:05 ` Sudakshina Das
@ 2018-03-21 8:59 ` Christophe Lyon
2018-03-21 12:01 ` Sudakshina Das
0 siblings, 1 reply; 6+ messages in thread
From: Christophe Lyon @ 2018-03-21 8:59 UTC (permalink / raw)
To: Sudakshina Das
Cc: Richard Earnshaw (lists),
gcc-patches, Kyrill Tkachov, Ramana Radhakrishnan, nd
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.
>
> 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?
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 }
>>> } */
>>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts
2018-03-21 8:59 ` Christophe Lyon
@ 2018-03-21 12:01 ` Sudakshina Das
2018-03-27 16:46 ` Sudakshina Das
0 siblings, 1 reply; 6+ messages in thread
From: Sudakshina Das @ 2018-03-21 12:01 UTC (permalink / raw)
To: Christophe Lyon
Cc: Richard Earnshaw (lists),
gcc-patches, Kyrill Tkachov, Ramana Radhakrishnan, nd
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.
>>
>> 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 }
>>>> } */
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts
2018-03-21 12:01 ` Sudakshina Das
@ 2018-03-27 16:46 ` Sudakshina Das
0 siblings, 0 replies; 6+ messages in thread
From: Sudakshina Das @ 2018-03-27 16:46 UTC (permalink / raw)
To: Christophe Lyon
Cc: Richard Earnshaw (lists),
gcc-patches, Kyrill Tkachov, Ramana Radhakrishnan, nd
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 }
>>>>> } */
>>>>>
>>>>
>>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-27 15:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 10:55 [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts 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 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).