[PATCH ARM] Fix PR target/63209

Xinliang David Li davidxl@google.com
Tue Sep 9 18:45:00 GMT 2014


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  <davidxl@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  <davidxl@google.com>

        PR target/63209
        * gcc.c-torture/execute/pr63209.c: New test



On Tue, Sep 9, 2014 at 10:31 AM, Richard Earnshaw <rearnsha@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  <davidxl@google.com>
>> +
>> +     PR target/63209
>> +     * config/arm/arm.md (movcond_addsi_1): New pattern.
>> +     (movcond_addsi): Add a constraint.
>> +
>>  2014-09-08  Trevor Saunders  <tsaunders@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  <davidxl@google.com>
>> +
>> +     PR target/63209
>> +     * gcc.c-torture/execute/pr63209.c: New test
>> +
>>  2014-09-08  Jakub Jelinek  <jakub@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;
>> +}
>> +
>> +
>> +
>>
>
>
-------------- next part --------------
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;
+}
+
+
+


More information about the Gcc-patches mailing list