Hi James,
On 04/02/16 13:49, James Greenhalgh wrote:
On Fri, Jan 29, 2016 at 02:27:34PM +0000, Kyrill Tkachov wrote:
Hi all,
In this PR we ICE during combine when trying to propagate a comparison into a
vec_duplicate,
that is we end up creating the rtx:
(vec_duplicate:V4SI (eq:CC_NZ (reg:CC_NZ 66 cc)
(const_int 0 [0])))
The documentation for vec_duplicate says:
"The output vector mode must have the same submodes as the input vector mode or the
scalar modes"
So this is invalid RTL, which triggers an assert in simplify-rtx to that effect.
It has been suggested on the PR that this is because we use a special_predicate
for
aarch64_comparison_operator which means that it ignores the mode when matching.
This is fine when used in RTXes that don't need it, like if_then_else
expressions
but can cause trouble when used in places where the modes do matter, like in
SET operations. In this particular ICE the cause was the conditional store
patterns that could end up matching an intermediate rtx during combine of
(set (reg:SI) (eq:CC_NZ x y)).
The suggested solution is to define a separate predicate with the same
conditions as aarch64_comparison_operator but make it not special, so it gets
automatic mode checks to prevent such a situation.
This patch does that.
Bootstrapped and tested on aarch64-linux-gnu.
SPEC2006 codegen did not change with this patch, so there shouldn't be
any code quality regressions.
Ok for trunk?
It would be good to leave a more detailed comment on
"aarch64_comparison_operator_mode" as to why we need it.
Otherwise, this is OK.
Ok, here's the patch with a comment added.
I'll commit it when the arm version is approved as well.
Thanks,
Kyrill
Thanks,
James
Thanks,
Kyrill
2016-01-29 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
PR target/69161
* config/aarch64/predicates.md (aarch64_comparison_operator_mode):
New predicate.
(aarch64_comparison_operator): Break overly long line into two.
(aarch64_comparison_operation): Likewise.
* config/aarch64/aarch64.md (cstorecc4): Use
aarch64_comparison_operator_mode instead of
aarch64_comparison_operator.
(cstore<mode>4): Likewise.
(aarch64_cstore<mode>): Likewise.
(*cstoresi_insn_uxtw): Likewise.
(cstore<mode>_neg): Likewise.
(*cstoresi_neg_uxtw): Likewise.
2016-01-29 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
PR target/69161
* gcc.c-torture/compile/pr69161.c: New test.
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 221f106430ea2c4a44352d937e660d0c1bbe10da..bf2140c5fd9458476d42e49100347dd05e1b21df 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3039,7 +3039,7 @@ (define_expand "cstore<mode>4"
(define_expand "cstorecc4"
[(set (match_operand:SI 0 "register_operand")
- (match_operator 1 "aarch64_comparison_operator"
+ (match_operator 1 "aarch64_comparison_operator_mode"
[(match_operand 2 "cc_register")
(match_operand 3 "const0_operand")]))]
""
@@ -3051,7 +3051,7 @@ (define_expand "cstorecc4"
(define_expand "cstore<mode>4"
[(set (match_operand:SI 0 "register_operand" "")
- (match_operator:SI 1 "aarch64_comparison_operator"
+ (match_operator:SI 1 "aarch64_comparison_operator_mode"
[(match_operand:GPF 2 "register_operand" "")
(match_operand:GPF 3 "aarch64_fp_compare_operand" "")]))]
""
@@ -3064,7 +3064,7 @@ (define_expand "cstore<mode>4"
(define_insn "aarch64_cstore<mode>"
[(set (match_operand:ALLI 0 "register_operand" "=r")
- (match_operator:ALLI 1 "aarch64_comparison_operator"
+ (match_operator:ALLI 1 "aarch64_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)]))]
""
"cset\\t%<w>0, %m1"
@@ -3109,7 +3109,7 @@ (define_insn_and_split "*compare_cstore<mode>_insn"
(define_insn "*cstoresi_insn_uxtw"
[(set (match_operand:DI 0 "register_operand" "=r")
(zero_extend:DI
- (match_operator:SI 1 "aarch64_comparison_operator"
+ (match_operator:SI 1 "aarch64_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
""
"cset\\t%w0, %m1"
@@ -3118,7 +3118,7 @@ (define_insn "*cstoresi_insn_uxtw"
(define_insn "cstore<mode>_neg"
[(set (match_operand:ALLI 0 "register_operand" "=r")
- (neg:ALLI (match_operator:ALLI 1 "aarch64_comparison_operator"
+ (neg:ALLI (match_operator:ALLI 1 "aarch64_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)])))]
""
"csetm\\t%<w>0, %m1"
@@ -3129,7 +3129,7 @@ (define_insn "cstore<mode>_neg"
(define_insn "*cstoresi_neg_uxtw"
[(set (match_operand:DI 0 "register_operand" "=r")
(zero_extend:DI
- (neg:SI (match_operator:SI 1 "aarch64_comparison_operator"
+ (neg:SI (match_operator:SI 1 "aarch64_comparison_operator_mode"
[(match_operand 2 "cc_register" "") (const_int 0)]))))]
""
"csetm\\t%w0, %m1"
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index e80e40683cada374303ea987017f95654531a032..11868278c3d0a1887c2065568f890c3eb8ff7f0b 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -229,10 +229,19 @@ (define_predicate "aarch64_reg_or_imm"
;; True for integer comparisons and for FP comparisons other than LTGT or UNEQ.
(define_special_predicate "aarch64_comparison_operator"
- (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,ordered,unlt,unle,unge,ungt"))
+ (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
+ ordered,unlt,unle,unge,ungt"))
+
+;; Same as aarch64_comparison_operator 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 "aarch64_comparison_operator_mode"
+ (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
+ ordered,unlt,unle,unge,ungt"))
(define_special_predicate "aarch64_comparison_operation"
- (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,ordered,unlt,unle,unge,ungt")
+ (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
+ ordered,unlt,unle,unge,ungt")
{
if (XEXP (op, 1) != const0_rtx)
return false;
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr69161.c b/gcc/testsuite/gcc.c-torture/compile/pr69161.c
new file mode 100644
index 0000000000000000000000000000000000000000..fdbb63f3335851c2d1537b098f245a9b85ef3695
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr69161.c
@@ -0,0 +1,19 @@
+/* PR target/69161. */
+
+char a;
+int b, c, d, e;
+
+void
+foo (void)
+{
+ int f;
+ for (f = 0; f <= 4; f++)
+ {
+ for (d = 0; d < 20; d++)
+ {
+ __INTPTR_TYPE__ g = (__INTPTR_TYPE__) & c;
+ b &= (0 != g) > e;
+ }
+ e &= a;
+ }
+}
