public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [ARM] Fix an internal error
@ 2010-12-13 23:56 Bernd Schmidt
  2010-12-14 14:05 ` Richard Earnshaw
  0 siblings, 1 reply; 3+ messages in thread
From: Bernd Schmidt @ 2010-12-13 23:56 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]

This fixes a crash while compiling a customer testcase (which I don't
know yet whether it can be posted) for ARM.  Going into combine, we have

(insn 166 165 167 20 10208.c:22 (set (reg:CC 24 cc)
        (compare:CC (reg:SI 141 [ D.2039 ])
            (const_int 78 [0x4e])))

(insn 167 166 169 20 10208.c:22 (set (reg:SI 276)
        (eq:SI (reg:CC 24 cc)
            (const_int 0 [0x0])))

(insn 169 167 170 20 10208.c:22 (set (reg:CC 24 cc)
        (compare:CC (reg:SI 141 [ D.2039 ])
            (const_int 110 [0x6e])))

(insn 170 169 172 20 10208.c:22 (set (reg:SI 278)
        (eq:SI (reg:CC 24 cc)
            (const_int 0 [0x0])))

(insn 172 170 173 20 10208.c:22 (set (reg:SI 279)
        (ior:SI (reg:SI 276)
            (reg:SI 278)))

(insn 173 172 174 20 10208.c:14 (set (reg/v:SI 136 [ cur_k ])
        (and:SI (reg:SI 279)
            (const_int 255 [0xff])))

(insn 174 173 175 20 10208.c:28 (set (reg:CC 24 cc)
        (compare:CC (reg/v:SI 136 [ cur_k ])
            (const_int 0 [0x0])))

(jump_insn 175 174 192 20 10208.c:28 (set (pc)
        (if_then_else (le (reg:CC 24 cc)
                (const_int 0 [0x0]))
            (label_ref 180)
            (pc)))

All this gets combined so that we finally end up with the following
pattern, i3 == insn 174:

(set (reg:CC 24 cc)
    (compare:CC (ior:SI (eq:SI (reg:SI 141 [ D.2039 ])
                (const_int 78 [0x4e]))
            (eq:SI (reg:SI 141 [ D.2039 ])
                (const_int 110 [0x6e])))
        (const_int 0 [0x0])))

We call arm_select_cc_mode, it returns CC_DEQmode, and we later crash in
get_arm_condition_code because we don't expect to see a LE comparison
with that mode.

Fixed with the following patch. The comment at the top of
arm_select_dominance_cc_mode states that "in all cases OP will be EQ or
NE", which matches the assert in get_arm_condition_code, but the caller
does not ensure it.

This looks suspiciously like we have a missed-optimization bug as well,
but I did not investigate it. In the following gimple, the
compare-and-jump ought to be optimized away.

  D.2040_26 = D.2039_25 == 110;
  D.2041_27 = D.2039_25 == 78;
  D.2042_28 = D.2041_27 | D.2040_26;
  cur_k_11 = (int) D.2042_28;
  if (cur_k_11 > 0)
    goto <bb 17>;
  else
    goto <bb 18>;

Ok?


Bernd

[-- Attachment #2: dominance-cc.diff --]
[-- Type: text/plain, Size: 980 bytes --]

	* config/arm/arm.c (arm_select_cc_mode): Before calling
	arm_select_dominance_cc_mode for AND or IOR operations, ensure
	that op is NE or EQ.

Index: ../../gcc/trunk-sgxx-4_5/gcc/config/arm/arm.c
===================================================================
--- ../../gcc/trunk-sgxx-4_5/gcc/config/arm/arm.c	(revision 307695)
+++ ../../gcc/trunk-sgxx-4_5/gcc/config/arm/arm.c	(working copy)
@@ -10686,12 +10686,14 @@ arm_select_cc_mode (enum rtx_code op, rt
 
   /* Alternate canonicalizations of the above.  These are somewhat cleaner.  */
   if (GET_CODE (x) == AND
+      && (op == EQ || op == NE)
       && COMPARISON_P (XEXP (x, 0))
       && COMPARISON_P (XEXP (x, 1)))
     return arm_select_dominance_cc_mode (XEXP (x, 0), XEXP (x, 1),
 					 DOM_CC_X_AND_Y);
 
   if (GET_CODE (x) == IOR
+      && (op == EQ || op == NE)
       && COMPARISON_P (XEXP (x, 0))
       && COMPARISON_P (XEXP (x, 1)))
     return arm_select_dominance_cc_mode (XEXP (x, 0), XEXP (x, 1),

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [ARM] Fix an internal error
  2010-12-13 23:56 [ARM] Fix an internal error Bernd Schmidt
@ 2010-12-14 14:05 ` Richard Earnshaw
  2010-12-17 15:12   ` Bernd Schmidt
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Earnshaw @ 2010-12-14 14:05 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches


On Mon, 2010-12-13 at 23:59 +0100, Bernd Schmidt wrote:
> This fixes a crash while compiling a customer testcase (which I don't
> know yet whether it can be posted) for ARM.  Going into combine, we have
> 
> (insn 166 165 167 20 10208.c:22 (set (reg:CC 24 cc)
>         (compare:CC (reg:SI 141 [ D.2039 ])
>             (const_int 78 [0x4e])))
> 
> (insn 167 166 169 20 10208.c:22 (set (reg:SI 276)
>         (eq:SI (reg:CC 24 cc)
>             (const_int 0 [0x0])))
> 
> (insn 169 167 170 20 10208.c:22 (set (reg:CC 24 cc)
>         (compare:CC (reg:SI 141 [ D.2039 ])
>             (const_int 110 [0x6e])))
> 
> (insn 170 169 172 20 10208.c:22 (set (reg:SI 278)
>         (eq:SI (reg:CC 24 cc)
>             (const_int 0 [0x0])))
> 
> (insn 172 170 173 20 10208.c:22 (set (reg:SI 279)
>         (ior:SI (reg:SI 276)
>             (reg:SI 278)))
> 
> (insn 173 172 174 20 10208.c:14 (set (reg/v:SI 136 [ cur_k ])
>         (and:SI (reg:SI 279)
>             (const_int 255 [0xff])))
> 
> (insn 174 173 175 20 10208.c:28 (set (reg:CC 24 cc)
>         (compare:CC (reg/v:SI 136 [ cur_k ])
>             (const_int 0 [0x0])))
> 
> (jump_insn 175 174 192 20 10208.c:28 (set (pc)
>         (if_then_else (le (reg:CC 24 cc)
>                 (const_int 0 [0x0]))
>             (label_ref 180)
>             (pc)))
> 
> All this gets combined so that we finally end up with the following
> pattern, i3 == insn 174:
> 
> (set (reg:CC 24 cc)
>     (compare:CC (ior:SI (eq:SI (reg:SI 141 [ D.2039 ])
>                 (const_int 78 [0x4e]))
>             (eq:SI (reg:SI 141 [ D.2039 ])
>                 (const_int 110 [0x6e])))
>         (const_int 0 [0x0])))
> 
> We call arm_select_cc_mode, it returns CC_DEQmode, and we later crash in
> get_arm_condition_code because we don't expect to see a LE comparison
> with that mode.
> 
> Fixed with the following patch. The comment at the top of
> arm_select_dominance_cc_mode states that "in all cases OP will be EQ or
> NE", which matches the assert in get_arm_condition_code, but the caller
> does not ensure it.
> 
> This looks suspiciously like we have a missed-optimization bug as well,
> but I did not investigate it. In the following gimple, the
> compare-and-jump ought to be optimized away.
> 
Maybe not optimized away, but certainly simplified to an equality
operation (> 0 is equivalent to != 0 given the domain of the values
assigned to cur_k_11).

>   D.2040_26 = D.2039_25 == 110;
>   D.2041_27 = D.2039_25 == 78;
>   D.2042_28 = D.2041_27 | D.2040_26;
>   cur_k_11 = (int) D.2042_28;
>   if (cur_k_11 > 0)
>     goto <bb 17>;
>   else
>     goto <bb 18>;
> 
> Ok?
> 

Yes.

R.



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [ARM] Fix an internal error
  2010-12-14 14:05 ` Richard Earnshaw
@ 2010-12-17 15:12   ` Bernd Schmidt
  0 siblings, 0 replies; 3+ messages in thread
From: Bernd Schmidt @ 2010-12-17 15:12 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On 12/14/2010 02:41 PM, Richard Earnshaw wrote:
> 
> On Mon, 2010-12-13 at 23:59 +0100, Bernd Schmidt wrote:
>> This fixes a crash while compiling a customer testcase (which I don't
>> know yet whether it can be posted) for ARM.  Going into combine, we have

>> Ok?
>>
> 
> Yes.

The customer agreed to let us use the testcase, but said they hope we
can mark it as provided by them. I've installed the following.


Bernd

[-- Attachment #2: dominance-cc2.diff --]
[-- Type: text/plain, Size: 3209 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 167979)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2010-12-17  Bernd Schmidt  <bernds@codesourcery.com>
+
+	* config/arm/arm.c (arm_select_cc_mode): Before calling
+	arm_select_dominance_cc_mode for AND or IOR operations, ensure
+	that op is NE or EQ.
+
 2010-12-17  Dodji Seketeli  <dodji@redhat.com>
 
 	* dwarf2out.c (gen_type_die_with_usage): Do not try to emit debug
Index: testsuite/gcc.c-torture/compile/20101217-1.c
===================================================================
--- testsuite/gcc.c-torture/compile/20101217-1.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/20101217-1.c	(revision 0)
@@ -0,0 +1,36 @@
+/* Testcase provided by HUAWEI.  */
+#include <stdio.h>
+int main()
+{
+        int cur_k;
+        int cur_j=0;
+        int cur_i=28;
+        unsigned char temp_data[8];
+        unsigned int Data_Size=20;
+
+        for (cur_k=0;cur_j<7;cur_j++,cur_i++) {
+                if (cur_j%2==0) {
+                        temp_data[cur_k++]=0;
+                }
+                if (cur_k==7) {
+                        for (;cur_k>0;cur_k--) {
+                                if (cur_k>2) {
+                                        if ((temp_data[7-cur_k]=='n' || temp_data[7-cur_k]=='N' ) && (temp_data[7-cur_k+1]=='a' || temp_data[7-cur_k+1]=='A' )) {
+                                                break;
+                                        }
+                                }
+                                if (cur_k==1) {
+                                        if (temp_data[7-cur_k]=='n' || temp_data[7-cur_k]=='N' ) {
+                                                break;
+                                        }
+                                }
+                        }
+                        if (cur_k==7) {
+                        } else {
+                                if (cur_k>0)
+                                        printf("dfjk");
+                        }
+                }
+        }
+return 0;
+}
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 167979)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2010-12-17  Bernd Schmidt  <bernds@codesourcery.com>
+
+	* gcc.c-torture/compile/20101217-1.c: New test.
+
 2010-12-17  Janus Weil  <janus@gcc.gnu.org>
 
 	PR fortran/46849
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 167979)
+++ config/arm/arm.c	(working copy)
@@ -10602,12 +10602,14 @@ arm_select_cc_mode (enum rtx_code op, rt
 
   /* Alternate canonicalizations of the above.  These are somewhat cleaner.  */
   if (GET_CODE (x) == AND
+      && (op == EQ || op == NE)
       && COMPARISON_P (XEXP (x, 0))
       && COMPARISON_P (XEXP (x, 1)))
     return arm_select_dominance_cc_mode (XEXP (x, 0), XEXP (x, 1),
 					 DOM_CC_X_AND_Y);
 
   if (GET_CODE (x) == IOR
+      && (op == EQ || op == NE)
       && COMPARISON_P (XEXP (x, 0))
       && COMPARISON_P (XEXP (x, 1)))
     return arm_select_dominance_cc_mode (XEXP (x, 0), XEXP (x, 1),

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-12-17 13:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-13 23:56 [ARM] Fix an internal error Bernd Schmidt
2010-12-14 14:05 ` Richard Earnshaw
2010-12-17 15:12   ` Bernd Schmidt

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).