Re: ICE on i386

Re: ICE on i386

[Jan Hubicka]

> 
> Honza,
> 
> we get with our current GCC compiler on i386:
> 
> slagsy.c: In function `slagsy_':
> slagsy.c:271: unrecognizable insn:
> (insn 2240 2234 1517 (set (reg:SI 610)
>         (ashift:SI (reg/v:SI 67)
>             (const_int 1 [0x1]))) -1 (nil)
>     (nil))
> slagsy.c:271: Internal compiler error in extract_insn, at recog.c:2158
> Please submit a full bug report,
> 

Hi,
the problem is that expand_expr calls simplifi_binary_operation on (plus reg
reg) that gets simplified into (shift reg 2) and expr happily returns it even
when it is supposed only return something that is operand.
Later the expression is passed to emit_move_insn and we get the wrong insn.

Honza

typedef int integer;
typedef int logical;

typedef char *address;
typedef short int shortint;
typedef float real;
typedef double doublereal;
typedef struct { real r, i; } complex;
typedef struct { doublereal r, i; } doublecomplex;
typedef short int shortlogical;
typedef char logical1;
typedef char integer1;



static integer c__3 = 3;
static integer c__1 = 1;
static real c_b12 = 0.f;
static real c_b19 = -1.f;
static real c_b26 = 1.f;

                 int slagsy_(integer *n, integer *k, real *d, real *a,
        integer *lda, integer *iseed, real *work, integer *info)
{

    integer a_dim1, a_offset, i__1, i__2, i__3;
    real r__1;


    double r_sign(real *, real *);


    extern int sger_(integer *, integer *, real *, real *,
            integer *, real *, integer *, real *, integer *);
    extern real sdot_(integer *, real *, integer *, real *, integer *),
            snrm2_(integer *, real *, integer *);
    static integer i, j;
    extern int ssyr2_(char *, integer *, real *, real *,
            integer *, real *, integer *, real *, integer *);
    static real alpha;
    extern int sscal_(integer *, real *, real *, integer *),
            sgemv_(char *, integer *, integer *, real *, real *, integer *,
            real *, integer *, real *, real *, integer *), saxpy_(
            integer *, real *, real *, integer *, real *, integer *), ssymv_(
            char *, integer *, real *, real *, integer *, real *, integer *,
            real *, real *, integer *);
    static real wa, wb, wn;
    extern int xerbla_(char *, integer *), slarnv_(
            integer *, integer *, integer *, real *);
    static real tau;
    --d;
    a_dim1 = *lda;
    a_offset = a_dim1 + 1;
    a -= a_offset;
    --iseed;
    --work;


    *info = 0;
    if (*n < 0) {
        *info = -1;
    } else if (*k < 0 || *k > *n - 1) {
        *info = -2;
    } else if (*lda < ((1) >= (*n) ? (1) : (*n))) {
        *info = -5;
    }
    if (*info < 0) {
        i__1 = -(*info);
        xerbla_("SLAGSY", &i__1);
        return 0;
    }



    i__1 = *n;
    for (j = 1; j <= i__1; ++j) {
        i__2 = *n;
        for (i = j + 1; i <= i__2; ++i) {
            a[i + j * a_dim1] = 0.f;

        }

    }
    i__1 = *n;
    for (i = 1; i <= i__1; ++i) {
        a[i + i * a_dim1] = d[i];

    }



    for (i = *n - 1; i >= 1; --i) {



        i__1 = *n - i + 1;
        slarnv_(&c__3, &iseed[1], &i__1, &work[1]);
        i__1 = *n - i + 1;
        wn = snrm2_(&i__1, &work[1], &c__1);
        wa = r_sign(&wn, &work[1]);
        if (wn == 0.f) {
            tau = 0.f;
        } else {
            wb = work[1] + wa;
            i__1 = *n - i;
            r__1 = 1.f / wb;
            sscal_(&i__1, &r__1, &work[2], &c__1);
            work[1] = 1.f;
            tau = wb / wa;
        }






        i__1 = *n - i + 1;
        ssymv_("Lower", &i__1, &tau, &a[i + i * a_dim1], lda, &work[1], &c__1,
                 &c_b12, &work[*n + 1], &c__1);



        i__1 = *n - i + 1;
        alpha = tau * -.5f * sdot_(&i__1, &work[*n + 1], &c__1, &work[1], &
                c__1);
        i__1 = *n - i + 1;
        saxpy_(&i__1, &alpha, &work[1], &c__1, &work[*n + 1], &c__1);



        i__1 = *n - i + 1;
        ssyr2_("Lower", &i__1, &c_b19, &work[1], &c__1, &work[*n + 1], &c__1,
                &a[i + i * a_dim1], lda);

    }



    i__1 = *n - 1 - *k;
    for (i = 1; i <= i__1; ++i) {



        i__2 = *n - *k - i + 1;
        wn = snrm2_(&i__2, &a[*k + i + i * a_dim1], &c__1);
        wa = r_sign(&wn, &a[*k + i + i * a_dim1]);
        if (wn == 0.f) {
            tau = 0.f;
        } else {
            wb = a[*k + i + i * a_dim1] + wa;
            i__2 = *n - *k - i;
            r__1 = 1.f / wb;
            sscal_(&i__2, &r__1, &a[*k + i + 1 + i * a_dim1], &c__1);
            a[*k + i + i * a_dim1] = 1.f;
            tau = wb / wa;
        }



        i__2 = *n - *k - i + 1;
        i__3 = *k - 1;
        sgemv_("Transpose", &i__2, &i__3, &c_b26, &a[*k + i + (i + 1) *
                a_dim1], lda, &a[*k + i + i * a_dim1], &c__1, &c_b12, &work[1]
                , &c__1);
        i__2 = *n - *k - i + 1;
        i__3 = *k - 1;
        r__1 = -(doublereal)tau;
        sger_(&i__2, &i__3, &r__1, &a[*k + i + i * a_dim1], &c__1, &work[1], &
                c__1, &a[*k + i + (i + 1) * a_dim1], lda);






        i__2 = *n - *k - i + 1;
        ssymv_("Lower", &i__2, &tau, &a[*k + i + (*k + i) * a_dim1], lda, &a[*
                k + i + i * a_dim1], &c__1, &c_b12, &work[1], &c__1);



        i__2 = *n - *k - i + 1;
        alpha = tau * -.5f * sdot_(&i__2, &work[1], &c__1, &a[*k + i + i *
                a_dim1], &c__1);
        i__2 = *n - *k - i + 1;
        saxpy_(&i__2, &alpha, &a[*k + i + i * a_dim1], &c__1, &work[1], &c__1)
                ;



        i__2 = *n - *k - i + 1;
        ssyr2_("Lower", &i__2, &c_b19, &a[*k + i + i * a_dim1], &c__1, &work[
                1], &c__1, &a[*k + i + (*k + i) * a_dim1], lda);

        a[*k + i + i * a_dim1] = -(doublereal)wa;
        i__2 = *n;
        for (j = *k + i + 1; j <= i__2; ++j) {
            a[j + i * a_dim1] = 0.f;

        }

    }



    i__1 = *n;
    for (j = 1; j <= i__1; ++j) {
        i__2 = *n;
        for (i = j + 1; i <= i__2; ++i) {
            a[j + i * a_dim1] = a[i + j * a_dim1];

        }

    }
    return 0;



}
Fri Oct 18 12:34:33 CEST 2002  Jan Hubicka  <jh@suse.cz>
	* expr.c (expand_expr): Do not return RTL expression.

Index: expr.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/expr.c,v
retrieving revision 1.488
diff -c -3 -p -r1.488 expr.c
*** expr.c	15 Oct 2002 20:09:32 -0000	1.488
--- expr.c	18 Oct 2002 10:34:08 -0000
*************** expand_expr (exp, target, tmode, modifie
*** 7912,7918 ****
  	  op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget, VOIDmode, 0);
  	  op1 = expand_expr (TREE_OPERAND (exp, 1), NULL_RTX, VOIDmode, 0);
  	  temp = simplify_binary_operation (PLUS, mode, op0, op1);
! 	  if (temp)
  	    return temp;
  	  goto binop2;
  	}
--- 7912,7918 ----
  	  op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget, VOIDmode, 0);
  	  op1 = expand_expr (TREE_OPERAND (exp, 1), NULL_RTX, VOIDmode, 0);
  	  temp = simplify_binary_operation (PLUS, mode, op0, op1);
! 	  if (temp && general_operand (temp, mode))
  	    return temp;
  	  goto binop2;
  	}

Re: ICE on i386

[Eric Botcazou]

> the problem is that expand_expr calls simplifi_binary_operation on (plus
> reg reg) that gets simplified into (shift reg 2) and expr happily returns
> it even when it is supposed only return something that is operand.
> Later the expression is passed to emit_move_insn and we get the wrong
> insn.

My fault. I introduced the call to simplify_binary_operation() in order to
fix PR c/7411: http://gcc.gnu.org/ml/gcc-patches/2002-09/msg01525.html

Perhaps simply discarding null operands would be more reasonable ?

--
Eric Botcazou

Re: ICE on i386

[Richard Henderson]

On Fri, Oct 18, 2002 at 12:42:57PM +0200, Jan Hubicka wrote:
>   	  temp = simplify_binary_operation (PLUS, mode, op0, op1);
> - 	  if (temp)
> + 	  if (temp && general_operand (temp, mode))
>   	    return temp;

Better, I think, would be to use force_operand.


r~

Re: ICE on i386

[Eric Botcazou]

> Better, I think, would be to use force_operand.

There is the following comment that seems to forbid using force_operand in
at least one case:

      /* No sense saving up arithmetic to be done
  if it's all in the wrong mode to form part of an address.
  And force_operand won't know whether to sign-extend or
  zero-extend.  */
      if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER)
   || mode != ptr_mode)
 {
   op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget, VOIDmode, 0);
   op1 = expand_expr (TREE_OPERAND (exp, 1), NULL_RTX, VOIDmode, 0);
   temp = simplify_binary_operation (PLUS, mode, op0, op1);
   if (temp)
     return temp;
   goto binop2;
 }

That's a little weird though, because force_operand is not used subsequently
in the PLUS_EXPR case.

Would you not want to consider to simply discard null operands instead of
calling simplify_binary_operation then general_operand or force_operand, in
light of the recent buzz around compilation time performances ? The
following patch:

--- gcc/expr.c.orig Fri Oct 25 22:00:59 2002
+++ gcc/expr.c Fri Oct 25 23:31:24 2002
@@ -7911,9 +7911,10 @@
  {
    op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget, VOIDmode, 0);
    op1 = expand_expr (TREE_OPERAND (exp, 1), NULL_RTX, VOIDmode, 0);
-   temp = simplify_binary_operation (PLUS, mode, op0, op1);
-   if (temp)
-     return temp;
+   if (op0 == const0_rtx)
+     return op1;
+   if (op1 == const0_rtx)
+     return op0;
    goto binop2;
  }

fixes both PR c/7411 (for which the call to simplify_binary_operation was
added) and the current ICE (now PR opt/8334) ?

--
Eric Botcazou

Re: ICE on i386

[Richard Henderson]

On Sat, Oct 26, 2002 at 12:41:20AM +0200, Eric Botcazou wrote:
> That's a little weird though, because force_operand is not used subsequently
> in the PLUS_EXPR case.
> 
> Would you not want to consider to simply discard null operands instead of
> calling simplify_binary_operation then general_operand or force_operand, in
> light of the recent buzz around compilation time performances ?

I guess you're right.

> The following patch:
[...]
> fixes both PR c/7411 (for which the call to simplify_binary_operation was
> added) and the current ICE (now PR opt/8334) ?

Applied.


r~

Re: ICE on i386

[Eric Botcazou]

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

> Applied.

Thanks. For the sake of completeness, bootstrapped and regtested 
(c,c++,objc,f77) on i586-redhat-linux-gnu.

It fixes PR optimization/8334. I've attached a patch against ChangeLog to 
record that and the unavoidable testcase.

-- 
Eric Botcazou

[-- Attachment #2: changelog.diff --]
[-- Type: text/x-diff, Size: 479 bytes --]

--- gcc/ChangeLog.orig	Wed Oct 30 08:28:08 2002
+++ gcc/ChangeLog	Wed Oct 30 08:28:28 2002
@@ -61,8 +61,9 @@
 
 2002-10-29  Eric Botcazou  <ebotcazou@libertysurf.fr>
 
-        * expr.c (expand_expr) [PLUS]: Don't use simplify_binary_operation;
-        check for zero operands explicitly.
+	PR optimization/8334
+	* expr.c (expand_expr) [PLUS]: Don't use simplify_binary_operation;
+	check for zero operands explicitly.
 
 2002-10-29  Richard Sandiford  <rsandifo@redhat.com>
 

[-- Attachment #3: pr8334.c --]
[-- Type: text/x-csrc, Size: 306 bytes --]

/* PR optimization/8334 */
/* Verify that GCC produces valid operands
   after simplifying an addition. */

void foo(int m, int n, double *f)
{
  int i, j, k = 1;

  for (j = 0; j < n; j++) {
    for (i = k; i < m; i++) {
      f[i] = (double) (i * j);
      f[i + j] = (double) ((i + 1) * j);
    }
  }
}