* [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns
@ 2016-01-29 14:28 Kyrill Tkachov
2016-02-05 9:50 ` Kyrill Tkachov
0 siblings, 1 reply; 5+ messages in thread
From: Kyrill Tkachov @ 2016-01-29 14:28 UTC (permalink / raw)
To: GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]
Hi all,
Similar to aarch64, the arm port also suffers from PR target/69161 when combine
tries to propagate a CCmode comparison into a vec_duplicate, creating invalid
RTL that ICEs.
Please refer to the PR and the aarch64 fix for more info.
The fix for arm is very similar.
We define a new predicate identical to arm_comparison_operator but
make it not special so that it gets the normal mode checks.
This prevents combine from matching an intermediate CCmode cstore
(where it's doing an SImode SET of a CCmode source) which it then
tries to propagate into a V4SImode vec_duplicate.
The offending patterns are the cstore patterns, so this patch updates them
to use the new predicate with mode checks. Both arm and thumb patterns
are updated.
There was no codegen difference observed on SPEC2006 for arm.
Bootstrapped and tested on arm-none-linux-gnueabihf.
Ok for trunk?
Thanks,
Kyrill
2016-01-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/69161
* config/arm/predicates.md (arm_comparison_operator_mode):
New predicate.
* config/arm/arm.md (*mov_scc): Use arm_comparison_operator_mode
instead of arm_comparison_operator.
(*mov_negscc): Likewise.
(*mov_notscc): Likewise.
* config/arm/thumb2.md (*thumb2_mov_scc): Likewise.
(*thumb2_mov_negscc): Likewise.
(*thumb2_mov_negscc_strict_it): Likewise.
(*thumb2_mov_notscc): Likewise.
(*thumb2_mov_notscc_strict_it): Likewise.
[-- Attachment #2: arm-cc-mode-cstore.patch --]
[-- Type: text/x-patch, Size: 4141 bytes --]
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 5129e858578dd3f3c3c46b089c96011f6a6423c3..15b4a4a1278c6be14dca1887aff8dc7c7a8fc16d 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -7190,7 +7190,7 @@ (define_expand "cstore_cc"
(define_insn_and_split "*mov_scc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (match_operator:SI 1 "arm_comparison_operator"
+ (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)]))]
"TARGET_ARM"
"#" ; "mov%D1\\t%0, #0\;mov%d1\\t%0, #1"
@@ -7207,7 +7207,7 @@ (define_insn_and_split "*mov_scc"
(define_insn_and_split "*mov_negscc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (neg:SI (match_operator:SI 1 "arm_comparison_operator"
+ (neg:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_ARM"
"#" ; "mov%D1\\t%0, #0\;mvn%d1\\t%0, #0"
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index c66c31d5c6047aa7decfe7e95d111d5fbf6fb52e..b8f09ab6b109f80abe2df08a8b7f954f521ec1bf 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -341,6 +341,11 @@ (define_special_predicate "arm_comparison_operator"
(and (match_operand 0 "expandable_comparison_operator")
(match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
+;; Likewise, but don't ignore the mode.
+(define_predicate "arm_comparison_operator_mode"
+ (and (match_operand 0 "expandable_comparison_operator")
+ (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
+
(define_special_predicate "lt_ge_comparison_operator"
(match_code "lt,ge"))
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 3e762018e4d3761852dda6434d3a8e31166a8678..7368d0658da1afde05b2dfc40eda79e1c228df1f 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -370,7 +370,7 @@ (define_insn "*thumb2_cmpsi_neg_shiftsi"
(define_insn_and_split "*thumb2_mov_scc"
[(set (match_operand:SI 0 "s_register_operand" "=l,r")
- (match_operator:SI 1 "arm_comparison_operator"
+ (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)]))]
"TARGET_THUMB2"
"#" ; "ite\\t%D1\;mov%D1\\t%0, #0\;mov%d1\\t%0, #1"
@@ -388,7 +388,7 @@ (define_insn_and_split "*thumb2_mov_scc"
(define_insn_and_split "*thumb2_mov_negscc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (neg:SI (match_operator:SI 1 "arm_comparison_operator"
+ (neg:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && !arm_restrict_it"
"#" ; "ite\\t%D1\;mov%D1\\t%0, #0\;mvn%d1\\t%0, #0"
@@ -407,7 +407,7 @@ (define_insn_and_split "*thumb2_mov_negscc"
(define_insn_and_split "*thumb2_mov_negscc_strict_it"
[(set (match_operand:SI 0 "low_register_operand" "=l")
- (neg:SI (match_operator:SI 1 "arm_comparison_operator"
+ (neg:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && arm_restrict_it"
"#" ; ";mvn\\t%0, #0 ;it\\t%D1\;mov%D1\\t%0, #0\"
@@ -436,7 +436,7 @@ (define_insn_and_split "*thumb2_mov_negscc_strict_it"
(define_insn_and_split "*thumb2_mov_notscc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (not:SI (match_operator:SI 1 "arm_comparison_operator"
+ (not:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && !arm_restrict_it"
"#" ; "ite\\t%D1\;mvn%D1\\t%0, #0\;mvn%d1\\t%0, #1"
@@ -456,7 +456,7 @@ (define_insn_and_split "*thumb2_mov_notscc"
(define_insn_and_split "*thumb2_mov_notscc_strict_it"
[(set (match_operand:SI 0 "low_register_operand" "=l")
- (not:SI (match_operator:SI 1 "arm_comparison_operator"
+ (not:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && arm_restrict_it"
"#" ; "mvn %0, #0 ; it%d1 ; lsl%d1 %0, %0, #1"
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns
2016-01-29 14:28 [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns Kyrill Tkachov
@ 2016-02-05 9:50 ` Kyrill Tkachov
2016-02-11 10:44 ` Kyrill Tkachov
0 siblings, 1 reply; 5+ messages in thread
From: Kyrill Tkachov @ 2016-02-05 9:50 UTC (permalink / raw)
To: GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
Ping
https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02309.html
Thanks,
Kyrill
On 29/01/16 14:27, Kyrill Tkachov wrote:
> Hi all,
>
> Similar to aarch64, the arm port also suffers from PR target/69161 when combine
> tries to propagate a CCmode comparison into a vec_duplicate, creating invalid
> RTL that ICEs.
> Please refer to the PR and the aarch64 fix for more info.
> The fix for arm is very similar.
> We define a new predicate identical to arm_comparison_operator but
> make it not special so that it gets the normal mode checks.
> This prevents combine from matching an intermediate CCmode cstore
> (where it's doing an SImode SET of a CCmode source) which it then
> tries to propagate into a V4SImode vec_duplicate.
>
> The offending patterns are the cstore patterns, so this patch updates them
> to use the new predicate with mode checks. Both arm and thumb patterns
> are updated.
>
> There was no codegen difference observed on SPEC2006 for arm.
> Bootstrapped and tested on arm-none-linux-gnueabihf.
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2016-01-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> PR target/69161
> * config/arm/predicates.md (arm_comparison_operator_mode):
> New predicate.
> * config/arm/arm.md (*mov_scc): Use arm_comparison_operator_mode
> instead of arm_comparison_operator.
> (*mov_negscc): Likewise.
> (*mov_notscc): Likewise.
> * config/arm/thumb2.md (*thumb2_mov_scc): Likewise.
> (*thumb2_mov_negscc): Likewise.
> (*thumb2_mov_negscc_strict_it): Likewise.
> (*thumb2_mov_notscc): Likewise.
> (*thumb2_mov_notscc_strict_it): Likewise.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns
2016-02-05 9:50 ` Kyrill Tkachov
@ 2016-02-11 10:44 ` Kyrill Tkachov
0 siblings, 0 replies; 5+ messages in thread
From: Kyrill Tkachov @ 2016-02-11 10:44 UTC (permalink / raw)
To: GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
Ping.
Thanks,
Kyrill
On 05/02/16 09:50, Kyrill Tkachov wrote:
> Ping
> https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02309.html
>
> Thanks,
> Kyrill
>
> On 29/01/16 14:27, Kyrill Tkachov wrote:
>> Hi all,
>>
>> Similar to aarch64, the arm port also suffers from PR target/69161 when combine
>> tries to propagate a CCmode comparison into a vec_duplicate, creating invalid
>> RTL that ICEs.
>> Please refer to the PR and the aarch64 fix for more info.
>> The fix for arm is very similar.
>> We define a new predicate identical to arm_comparison_operator but
>> make it not special so that it gets the normal mode checks.
>> This prevents combine from matching an intermediate CCmode cstore
>> (where it's doing an SImode SET of a CCmode source) which it then
>> tries to propagate into a V4SImode vec_duplicate.
>>
>> The offending patterns are the cstore patterns, so this patch updates them
>> to use the new predicate with mode checks. Both arm and thumb patterns
>> are updated.
>>
>> There was no codegen difference observed on SPEC2006 for arm.
>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2016-01-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>
>> PR target/69161
>> * config/arm/predicates.md (arm_comparison_operator_mode):
>> New predicate.
>> * config/arm/arm.md (*mov_scc): Use arm_comparison_operator_mode
>> instead of arm_comparison_operator.
>> (*mov_negscc): Likewise.
>> (*mov_notscc): Likewise.
>> * config/arm/thumb2.md (*thumb2_mov_scc): Likewise.
>> (*thumb2_mov_negscc): Likewise.
>> (*thumb2_mov_negscc_strict_it): Likewise.
>> (*thumb2_mov_notscc): Likewise.
>> (*thumb2_mov_notscc_strict_it): Likewise.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns
2016-02-17 13:13 Nick Clifton
@ 2016-02-17 13:40 ` Kyrill Tkachov
0 siblings, 0 replies; 5+ messages in thread
From: Kyrill Tkachov @ 2016-02-17 13:40 UTC (permalink / raw)
To: Nick Clifton; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]
Hi Nick,
On 17/02/16 13:13, Nick Clifton wrote:
> Hi Kyrill,
>
>> Ok for trunk?
>>
>> 2016-01-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>
>> PR target/69161
>> * config/arm/predicates.md (arm_comparison_operator_mode):
>> New predicate.
>> * config/arm/arm.md (*mov_scc): Use arm_comparison_operator_mode
>> instead of arm_comparison_operator.
>> (*mov_negscc): Likewise.
>> (*mov_notscc): Likewise.
>> * config/arm/thumb2.md (*thumb2_mov_scc): Likewise.
>> (*thumb2_mov_negscc): Likewise.
>> (*thumb2_mov_negscc_strict_it): Likewise.
>> (*thumb2_mov_notscc): Likewise.
>> (*thumb2_mov_notscc_strict_it): Likewise.
> Approved - please apply - but ...
>
Thanks!
>> diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
>> index c66c31d5c6047aa7decfe7e95d111d5fbf6fb52e..b8f09ab6b109f80abe2df08a8b7f954f521ec1bf 100644
>> --- a/gcc/config/arm/predicates.md
>> +++ b/gcc/config/arm/predicates.md
>> @@ -341,6 +341,11 @@ (define_special_predicate "arm_comparison_operator"
>> (and (match_operand 0 "expandable_comparison_operator")
>> (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
>>
>> +;; Likewise, but don't ignore the mode.
>> +(define_predicate "arm_comparison_operator_mode"
> Please could you extend the comment here to reference the PR. That way
> anyone reading this code who wonders why we need to have two versions of
> the same predicate will be able understand what is happening.
Ok, here's what I committed with r233495.
Kyrill
> Cheers
> Nick
>
[-- Attachment #2: arm-comp-mode.patch --]
[-- Type: text/x-patch, Size: 4330 bytes --]
commit 59380f7f3e34f4c4e17a610e67341a0de0272c15
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Wed Jan 13 13:29:36 2016 +0000
[ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore patterns
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 5129e85..15b4a4a 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -7190,7 +7190,7 @@ (define_expand "cstore_cc"
(define_insn_and_split "*mov_scc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (match_operator:SI 1 "arm_comparison_operator"
+ (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)]))]
"TARGET_ARM"
"#" ; "mov%D1\\t%0, #0\;mov%d1\\t%0, #1"
@@ -7207,7 +7207,7 @@ (define_insn_and_split "*mov_scc"
(define_insn_and_split "*mov_negscc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (neg:SI (match_operator:SI 1 "arm_comparison_operator"
+ (neg:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_ARM"
"#" ; "mov%D1\\t%0, #0\;mvn%d1\\t%0, #0"
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index c66c31d..f696458 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -341,6 +341,13 @@ (define_special_predicate "arm_comparison_operator"
(and (match_operand 0 "expandable_comparison_operator")
(match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
+;; Likewise, but don't ignore the mode.
+;; RTL SET operations require their operands source and destination have
+;; the same modes, so we can't ignore the modes there. See PR target/69161.
+(define_predicate "arm_comparison_operator_mode"
+ (and (match_operand 0 "expandable_comparison_operator")
+ (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
+
(define_special_predicate "lt_ge_comparison_operator"
(match_code "lt,ge"))
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 39a3d80..9925365 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -370,7 +370,7 @@ (define_insn "*thumb2_cmpsi_neg_shiftsi"
(define_insn_and_split "*thumb2_mov_scc"
[(set (match_operand:SI 0 "s_register_operand" "=l,r")
- (match_operator:SI 1 "arm_comparison_operator"
+ (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)]))]
"TARGET_THUMB2"
"#" ; "ite\\t%D1\;mov%D1\\t%0, #0\;mov%d1\\t%0, #1"
@@ -388,7 +388,7 @@ (define_insn_and_split "*thumb2_mov_scc"
(define_insn_and_split "*thumb2_mov_negscc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (neg:SI (match_operator:SI 1 "arm_comparison_operator"
+ (neg:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && !arm_restrict_it"
"#" ; "ite\\t%D1\;mov%D1\\t%0, #0\;mvn%d1\\t%0, #0"
@@ -407,7 +407,7 @@ (define_insn_and_split "*thumb2_mov_negscc"
(define_insn_and_split "*thumb2_mov_negscc_strict_it"
[(set (match_operand:SI 0 "low_register_operand" "=l")
- (neg:SI (match_operator:SI 1 "arm_comparison_operator"
+ (neg:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && arm_restrict_it"
"#" ; ";mvn\\t%0, #0 ;it\\t%D1\;mov%D1\\t%0, #0\"
@@ -436,7 +436,7 @@ (define_insn_and_split "*thumb2_mov_negscc_strict_it"
(define_insn_and_split "*thumb2_mov_notscc"
[(set (match_operand:SI 0 "s_register_operand" "=r")
- (not:SI (match_operator:SI 1 "arm_comparison_operator"
+ (not:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && !arm_restrict_it"
"#" ; "ite\\t%D1\;mvn%D1\\t%0, #0\;mvn%d1\\t%0, #1"
@@ -456,7 +456,7 @@ (define_insn_and_split "*thumb2_mov_notscc"
(define_insn_and_split "*thumb2_mov_notscc_strict_it"
[(set (match_operand:SI 0 "low_register_operand" "=l")
- (not:SI (match_operator:SI 1 "arm_comparison_operator"
+ (not:SI (match_operator:SI 1 "arm_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
"TARGET_THUMB2 && arm_restrict_it"
"#" ; "mvn %0, #0 ; it%d1 ; lsl%d1 %0, %0, #1"
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns
@ 2016-02-17 13:13 Nick Clifton
2016-02-17 13:40 ` Kyrill Tkachov
0 siblings, 1 reply; 5+ messages in thread
From: Nick Clifton @ 2016-02-17 13:13 UTC (permalink / raw)
To: kyrylo.tkachov; +Cc: gcc-patches
Hi Kyrill,
> Ok for trunk?
>
> 2016-01-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> PR target/69161
> * config/arm/predicates.md (arm_comparison_operator_mode):
> New predicate.
> * config/arm/arm.md (*mov_scc): Use arm_comparison_operator_mode
> instead of arm_comparison_operator.
> (*mov_negscc): Likewise.
> (*mov_notscc): Likewise.
> * config/arm/thumb2.md (*thumb2_mov_scc): Likewise.
> (*thumb2_mov_negscc): Likewise.
> (*thumb2_mov_negscc_strict_it): Likewise.
> (*thumb2_mov_notscc): Likewise.
> (*thumb2_mov_notscc_strict_it): Likewise.
Approved - please apply - but ...
> diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
> index c66c31d5c6047aa7decfe7e95d111d5fbf6fb52e..b8f09ab6b109f80abe2df08a8b7f954f521ec1bf 100644
> --- a/gcc/config/arm/predicates.md
> +++ b/gcc/config/arm/predicates.md
> @@ -341,6 +341,11 @@ (define_special_predicate "arm_comparison_operator"
> (and (match_operand 0 "expandable_comparison_operator")
> (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
>
> +;; Likewise, but don't ignore the mode.
> +(define_predicate "arm_comparison_operator_mode"
Please could you extend the comment here to reference the PR. That way
anyone reading this code who wonders why we need to have two versions of
the same predicate will be able understand what is happening.
Cheers
Nick
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-17 13:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-29 14:28 [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns Kyrill Tkachov
2016-02-05 9:50 ` Kyrill Tkachov
2016-02-11 10:44 ` Kyrill Tkachov
2016-02-17 13:13 Nick Clifton
2016-02-17 13:40 ` Kyrill Tkachov
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).