Re: [PATCH ARM] Fix PR target/63209
With gcc regression test, no regressions are found.
David
On Tue, Sep 9, 2014 at 11:45 AM, Xinliang David Li davi...@google.com wrote:
Richard, thanks for the review. The revised patch is attached. Is this
one OK (after testing is done)?
David
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* config/arm/arm.md (movcond_addsi): Handle case where source
and target operands are the same
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* gcc.c-torture/execute/pr63209.c: New test
On Tue, Sep 9, 2014 at 10:31 AM, Richard Earnshaw rearn...@arm.com wrote:
On 09/09/14 16:58, Xinliang David Li wrote:
The attached patch fixed the problem reported in PR63209. The problem
exists in both gcc4.9 and trunk.
Regression test on arm-unknown-linux-gnueabi passes.
Ok for gcc-4.9 and trunk?
No, this isn't right. I don't think you need a new pattern here (you
certainly don't want a new insn - at most you would just need a new
split). But I think you just need some special case code in the
existing pattern to handle the overlap case.
Also, please do not post ChangeLog entries in patch format; they won't
apply by the time the patch is ready to be committed.
R.
(I sent the patch last night, but it got lost somehow)
David
pr63209.txt
Index: ChangeLog
===
--- ChangeLog (revision 215039)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2014-09-08 Xinliang David Li davi...@google.com
+
+ PR target/63209
+ * config/arm/arm.md (movcond_addsi_1): New pattern.
+ (movcond_addsi): Add a constraint.
+
2014-09-08 Trevor Saunders tsaund...@mozilla.com
* common/config/picochip/picochip-common.c: Remove.
Index: config/arm/arm.md
===
--- config/arm/arm.md (revision 215039)
+++ config/arm/arm.md (working copy)
@@ -9302,6 +9302,43 @@
(set_attr type multiple)]
)
+(define_insn_and_split movcond_addsi_1
+ [(set (match_operand:SI 0 s_register_operand =r,l,r)
+(if_then_else:SI
+ (match_operator 4 comparison_operator
+ [(plus:SI (match_operand:SI 2 s_register_operand r,r,r)
+(match_operand:SI 3 arm_add_operand rIL,rIL,rIL))
+ (const_int 0)])
+ (match_operand:SI 1 arm_rhs_operand rI,rPy,r)
+ (match_dup:SI 0)))
+(clobber (reg:CC CC_REGNUM))]
+TARGET_32BIT
+#
+ reload_completed
+ [(set (reg:CC_NOOV CC_REGNUM)
+(compare:CC_NOOV
+ (plus:SI (match_dup 2)
+ (match_dup 3))
+ (const_int 0)))
+(cond_exec (match_dup 5)
+ (set (match_dup 0) (match_dup 1)))]
+
+ {
+ enum machine_mode mode = SELECT_CC_MODE (GET_CODE (operands[4]),
+ operands[2], operands[3]);
+ enum rtx_code rc = GET_CODE (operands[4]);
+
+ operands[5] = gen_rtx_REG (mode, CC_REGNUM);
+ gcc_assert (!(mode == CCFPmode || mode == CCFPEmode));
+ operands[5] = gen_rtx_fmt_ee (rc, VOIDmode, operands[5], const0_rtx);
+ }
+
+ [(set_attr conds clob)
+ (set_attr enabled_for_depr_it no,yes,yes)
+ (set_attr type multiple)]
+)
+
+
(define_insn_and_split movcond_addsi
[(set (match_operand:SI 0 s_register_operand =r,l,r)
(if_then_else:SI
@@ -9312,7 +9349,7 @@
(match_operand:SI 1 arm_rhs_operand rI,rPy,r)
(match_operand:SI 2 arm_rhs_operand rI,rPy,r)))
(clobber (reg:CC CC_REGNUM))]
- TARGET_32BIT
+ TARGET_32BIT REGNO (operands[2]) != REGNO (operands[0])
#
reload_completed
[(set (reg:CC_NOOV CC_REGNUM)
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 215039)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2014-09-08 Xinliang David Li davi...@google.com
+
+ PR target/63209
+ * gcc.c-torture/execute/pr63209.c: New test
+
2014-09-08 Jakub Jelinek ja...@redhat.com
PR tree-optimization/60196
Index: testsuite/gcc.c-torture/execute/pr63209.c
===
--- testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
+++ testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
@@ -0,0 +1,27 @@
+static int Sub(int a, int b) {
+ return b -a;
+}
+
+static unsigned Select(unsigned a, unsigned b, unsigned c) {
+ const int pa_minus_pb =
+ Sub((a 8) 0xff, (b 8) 0xff) +
+ Sub((a 0) 0xff, (b 0) 0xff);
+ return (pa_minus_pb = 0) ? a : b;
+}
+
+__attribute__((noinline)) unsigned Predictor(unsigned left, const
unsigned* const top) {
+ const unsigned pred = Select(top[1], left, top[0]);
+ return pred;
+}
+
+int main(void) {
+ const unsigned top[2] = {0xff7a7a7a, 0xff7a7a7a};
+ const unsigned
Re: [PATCH ARM] Fix PR target/63209
On 09/09/14 19:45, Xinliang David Li wrote:
Richard, thanks for the review. The revised patch is attached. Is this
one OK (after testing is done)?
David
OK, but ...
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* config/arm/arm.md (movcond_addsi): Handle case where source
and target operands are the same
Full stop at end of sentence.
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* gcc.c-torture/execute/pr63209.c: New test
Likewise.
pr63209.txt
Index: config/arm/arm.md
===
--- config/arm/arm.md (revision 215039)
+++ config/arm/arm.md (working copy)
@@ -9328,10 +9328,16 @@
enum machine_mode mode = SELECT_CC_MODE (GET_CODE (operands[5]),
operands[3], operands[4]);
enum rtx_code rc = GET_CODE (operands[5]);
-
operands[6] = gen_rtx_REG (mode, CC_REGNUM);
gcc_assert (!(mode == CCFPmode || mode == CCFPEmode));
-rc = reverse_condition (rc);
+if (REGNO (operands[2]) != REGNO (operands[0]))
+ rc = reverse_condition (rc);
+else
+ {
+rtx tmp = operands[1];
+ operands[1] = operands[2];
+ operands[2] = tmp;
Please use tabs and white space consistently (use tabs). Also, you seem
to have some trailing white space at the end of some lines.
+ }
operands[6] = gen_rtx_fmt_ee (rc, VOIDmode, operands[6], const0_rtx);
}
Index: testsuite/gcc.c-torture/execute/pr63209.c
===
--- testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
+++ testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
@@ -0,0 +1,27 @@
+static int Sub(int a, int b) {
+ return b -a;
+}
+
+static unsigned Select(unsigned a, unsigned b, unsigned c) {
+ const int pa_minus_pb =
+ Sub((a 8) 0xff, (b 8) 0xff) +
+ Sub((a 0) 0xff, (b 0) 0xff);
+ return (pa_minus_pb = 0) ? a : b;
+}
+
+__attribute__((noinline)) unsigned Predictor(unsigned left, const unsigned*
const top) {
+ const unsigned pred = Select(top[1], left, top[0]);
+ return pred;
+}
+
+int main(void) {
+ const unsigned top[2] = {0xff7a7a7a, 0xff7a7a7a};
+ const unsigned left = 0xff7b7b7b;
+ const unsigned pred = Predictor(left, top /*+ 1*/);
+ if (pred == left)
+return 0;
+ return 1;
+}
+
+
+
Re: [PATCH ARM] Fix PR target/63209
Fixed the formatting and committed it to trunk: r215136. Will backport
it to gcc-4_9 branch.
thanks,
David
On Wed, Sep 10, 2014 at 11:24 AM, Richard Earnshaw rearn...@arm.com wrote:
On 09/09/14 19:45, Xinliang David Li wrote:
Richard, thanks for the review. The revised patch is attached. Is this
one OK (after testing is done)?
David
OK, but ...
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* config/arm/arm.md (movcond_addsi): Handle case where source
and target operands are the same
Full stop at end of sentence.
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* gcc.c-torture/execute/pr63209.c: New test
Likewise.
pr63209.txt
Index: config/arm/arm.md
===
--- config/arm/arm.md (revision 215039)
+++ config/arm/arm.md (working copy)
@@ -9328,10 +9328,16 @@
enum machine_mode mode = SELECT_CC_MODE (GET_CODE (operands[5]),
operands[3], operands[4]);
enum rtx_code rc = GET_CODE (operands[5]);
-
operands[6] = gen_rtx_REG (mode, CC_REGNUM);
gcc_assert (!(mode == CCFPmode || mode == CCFPEmode));
-rc = reverse_condition (rc);
+if (REGNO (operands[2]) != REGNO (operands[0]))
+ rc = reverse_condition (rc);
+else
+ {
+rtx tmp = operands[1];
+ operands[1] = operands[2];
+ operands[2] = tmp;
Please use tabs and white space consistently (use tabs). Also, you seem
to have some trailing white space at the end of some lines.
+ }
operands[6] = gen_rtx_fmt_ee (rc, VOIDmode, operands[6], const0_rtx);
}
Index: testsuite/gcc.c-torture/execute/pr63209.c
===
--- testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
+++ testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
@@ -0,0 +1,27 @@
+static int Sub(int a, int b) {
+ return b -a;
+}
+
+static unsigned Select(unsigned a, unsigned b, unsigned c) {
+ const int pa_minus_pb =
+ Sub((a 8) 0xff, (b 8) 0xff) +
+ Sub((a 0) 0xff, (b 0) 0xff);
+ return (pa_minus_pb = 0) ? a : b;
+}
+
+__attribute__((noinline)) unsigned Predictor(unsigned left, const unsigned*
const top) {
+ const unsigned pred = Select(top[1], left, top[0]);
+ return pred;
+}
+
+int main(void) {
+ const unsigned top[2] = {0xff7a7a7a, 0xff7a7a7a};
+ const unsigned left = 0xff7b7b7b;
+ const unsigned pred = Predictor(left, top /*+ 1*/);
+ if (pred == left)
+return 0;
+ return 1;
+}
+
+
+
Re: [PATCH ARM] Fix PR target/63209
On 09/09/14 16:58, Xinliang David Li wrote:
The attached patch fixed the problem reported in PR63209. The problem
exists in both gcc4.9 and trunk.
Regression test on arm-unknown-linux-gnueabi passes.
Ok for gcc-4.9 and trunk?
No, this isn't right. I don't think you need a new pattern here (you
certainly don't want a new insn - at most you would just need a new
split). But I think you just need some special case code in the
existing pattern to handle the overlap case.
Also, please do not post ChangeLog entries in patch format; they won't
apply by the time the patch is ready to be committed.
R.
(I sent the patch last night, but it got lost somehow)
David
pr63209.txt
Index: ChangeLog
===
--- ChangeLog (revision 215039)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2014-09-08 Xinliang David Li davi...@google.com
+
+ PR target/63209
+ * config/arm/arm.md (movcond_addsi_1): New pattern.
+ (movcond_addsi): Add a constraint.
+
2014-09-08 Trevor Saunders tsaund...@mozilla.com
* common/config/picochip/picochip-common.c: Remove.
Index: config/arm/arm.md
===
--- config/arm/arm.md (revision 215039)
+++ config/arm/arm.md (working copy)
@@ -9302,6 +9302,43 @@
(set_attr type multiple)]
)
+(define_insn_and_split movcond_addsi_1
+ [(set (match_operand:SI 0 s_register_operand =r,l,r)
+(if_then_else:SI
+ (match_operator 4 comparison_operator
+ [(plus:SI (match_operand:SI 2 s_register_operand r,r,r)
+(match_operand:SI 3 arm_add_operand rIL,rIL,rIL))
+ (const_int 0)])
+ (match_operand:SI 1 arm_rhs_operand rI,rPy,r)
+ (match_dup:SI 0)))
+(clobber (reg:CC CC_REGNUM))]
+TARGET_32BIT
+#
+ reload_completed
+ [(set (reg:CC_NOOV CC_REGNUM)
+(compare:CC_NOOV
+ (plus:SI (match_dup 2)
+ (match_dup 3))
+ (const_int 0)))
+(cond_exec (match_dup 5)
+ (set (match_dup 0) (match_dup 1)))]
+
+ {
+ enum machine_mode mode = SELECT_CC_MODE (GET_CODE (operands[4]),
+ operands[2], operands[3]);
+ enum rtx_code rc = GET_CODE (operands[4]);
+
+ operands[5] = gen_rtx_REG (mode, CC_REGNUM);
+ gcc_assert (!(mode == CCFPmode || mode == CCFPEmode));
+ operands[5] = gen_rtx_fmt_ee (rc, VOIDmode, operands[5], const0_rtx);
+ }
+
+ [(set_attr conds clob)
+ (set_attr enabled_for_depr_it no,yes,yes)
+ (set_attr type multiple)]
+)
+
+
(define_insn_and_split movcond_addsi
[(set (match_operand:SI 0 s_register_operand =r,l,r)
(if_then_else:SI
@@ -9312,7 +9349,7 @@
(match_operand:SI 1 arm_rhs_operand rI,rPy,r)
(match_operand:SI 2 arm_rhs_operand rI,rPy,r)))
(clobber (reg:CC CC_REGNUM))]
- TARGET_32BIT
+ TARGET_32BIT REGNO (operands[2]) != REGNO (operands[0])
#
reload_completed
[(set (reg:CC_NOOV CC_REGNUM)
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 215039)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2014-09-08 Xinliang David Li davi...@google.com
+
+ PR target/63209
+ * gcc.c-torture/execute/pr63209.c: New test
+
2014-09-08 Jakub Jelinek ja...@redhat.com
PR tree-optimization/60196
Index: testsuite/gcc.c-torture/execute/pr63209.c
===
--- testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
+++ testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
@@ -0,0 +1,27 @@
+static int Sub(int a, int b) {
+ return b -a;
+}
+
+static unsigned Select(unsigned a, unsigned b, unsigned c) {
+ const int pa_minus_pb =
+ Sub((a 8) 0xff, (b 8) 0xff) +
+ Sub((a 0) 0xff, (b 0) 0xff);
+ return (pa_minus_pb = 0) ? a : b;
+}
+
+__attribute__((noinline)) unsigned Predictor(unsigned left, const unsigned*
const top) {
+ const unsigned pred = Select(top[1], left, top[0]);
+ return pred;
+}
+
+int main(void) {
+ const unsigned top[2] = {0xff7a7a7a, 0xff7a7a7a};
+ const unsigned left = 0xff7b7b7b;
+ const unsigned pred = Predictor(left, top /*+ 1*/);
+ if (pred == left)
+return 0;
+ return 1;
+}
+
+
+
Re: [PATCH ARM] Fix PR target/63209
Richard, thanks for the review. The revised patch is attached. Is this
one OK (after testing is done)?
David
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* config/arm/arm.md (movcond_addsi): Handle case where source
and target operands are the same
2014-09-08 Xinliang David Li davi...@google.com
PR target/63209
* gcc.c-torture/execute/pr63209.c: New test
On Tue, Sep 9, 2014 at 10:31 AM, Richard Earnshaw rearn...@arm.com wrote:
On 09/09/14 16:58, Xinliang David Li wrote:
The attached patch fixed the problem reported in PR63209. The problem
exists in both gcc4.9 and trunk.
Regression test on arm-unknown-linux-gnueabi passes.
Ok for gcc-4.9 and trunk?
No, this isn't right. I don't think you need a new pattern here (you
certainly don't want a new insn - at most you would just need a new
split). But I think you just need some special case code in the
existing pattern to handle the overlap case.
Also, please do not post ChangeLog entries in patch format; they won't
apply by the time the patch is ready to be committed.
R.
(I sent the patch last night, but it got lost somehow)
David
pr63209.txt
Index: ChangeLog
===
--- ChangeLog (revision 215039)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2014-09-08 Xinliang David Li davi...@google.com
+
+ PR target/63209
+ * config/arm/arm.md (movcond_addsi_1): New pattern.
+ (movcond_addsi): Add a constraint.
+
2014-09-08 Trevor Saunders tsaund...@mozilla.com
* common/config/picochip/picochip-common.c: Remove.
Index: config/arm/arm.md
===
--- config/arm/arm.md (revision 215039)
+++ config/arm/arm.md (working copy)
@@ -9302,6 +9302,43 @@
(set_attr type multiple)]
)
+(define_insn_and_split movcond_addsi_1
+ [(set (match_operand:SI 0 s_register_operand =r,l,r)
+(if_then_else:SI
+ (match_operator 4 comparison_operator
+ [(plus:SI (match_operand:SI 2 s_register_operand r,r,r)
+(match_operand:SI 3 arm_add_operand rIL,rIL,rIL))
+ (const_int 0)])
+ (match_operand:SI 1 arm_rhs_operand rI,rPy,r)
+ (match_dup:SI 0)))
+(clobber (reg:CC CC_REGNUM))]
+TARGET_32BIT
+#
+ reload_completed
+ [(set (reg:CC_NOOV CC_REGNUM)
+(compare:CC_NOOV
+ (plus:SI (match_dup 2)
+ (match_dup 3))
+ (const_int 0)))
+(cond_exec (match_dup 5)
+ (set (match_dup 0) (match_dup 1)))]
+
+ {
+ enum machine_mode mode = SELECT_CC_MODE (GET_CODE (operands[4]),
+ operands[2], operands[3]);
+ enum rtx_code rc = GET_CODE (operands[4]);
+
+ operands[5] = gen_rtx_REG (mode, CC_REGNUM);
+ gcc_assert (!(mode == CCFPmode || mode == CCFPEmode));
+ operands[5] = gen_rtx_fmt_ee (rc, VOIDmode, operands[5], const0_rtx);
+ }
+
+ [(set_attr conds clob)
+ (set_attr enabled_for_depr_it no,yes,yes)
+ (set_attr type multiple)]
+)
+
+
(define_insn_and_split movcond_addsi
[(set (match_operand:SI 0 s_register_operand =r,l,r)
(if_then_else:SI
@@ -9312,7 +9349,7 @@
(match_operand:SI 1 arm_rhs_operand rI,rPy,r)
(match_operand:SI 2 arm_rhs_operand rI,rPy,r)))
(clobber (reg:CC CC_REGNUM))]
- TARGET_32BIT
+ TARGET_32BIT REGNO (operands[2]) != REGNO (operands[0])
#
reload_completed
[(set (reg:CC_NOOV CC_REGNUM)
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 215039)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2014-09-08 Xinliang David Li davi...@google.com
+
+ PR target/63209
+ * gcc.c-torture/execute/pr63209.c: New test
+
2014-09-08 Jakub Jelinek ja...@redhat.com
PR tree-optimization/60196
Index: testsuite/gcc.c-torture/execute/pr63209.c
===
--- testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
+++ testsuite/gcc.c-torture/execute/pr63209.c (revision 0)
@@ -0,0 +1,27 @@
+static int Sub(int a, int b) {
+ return b -a;
+}
+
+static unsigned Select(unsigned a, unsigned b, unsigned c) {
+ const int pa_minus_pb =
+ Sub((a 8) 0xff, (b 8) 0xff) +
+ Sub((a 0) 0xff, (b 0) 0xff);
+ return (pa_minus_pb = 0) ? a : b;
+}
+
+__attribute__((noinline)) unsigned Predictor(unsigned left, const unsigned*
const top) {
+ const unsigned pred = Select(top[1], left, top[0]);
+ return pred;
+}
+
+int main(void) {
+ const unsigned top[2] = {0xff7a7a7a, 0xff7a7a7a};
+ const unsigned left = 0xff7b7b7b;
+ const unsigned pred = Predictor(left, top /*+ 1*/);
+ if (pred == left)
+return 0;
+ return 1;
+}
+
+
+
