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;
>> +}
>> +
>> +
>> +
>>
>
>
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;
+ }
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;
+}
+
+
+
