Patchwork [ARM] Fix an internal error

login
register
mail settings
Submitter Bernd Schmidt
Date Dec. 13, 2010, 10:59 p.m.
Message ID <4D06A565.4030103@codesourcery.com>
Download mbox | patch
Permalink /patch/75434/
State New
Headers show

Comments

Bernd Schmidt - Dec. 13, 2010, 10:59 p.m.
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
* 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.
Richard Earnshaw - Dec. 14, 2010, 1:41 p.m.
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.

Patch

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