Bug 32682 - [4.3 Regression] ICE in gfc_trans_array_constructor, at fortran/trans-array.c:1664
[4.3 Regression] ICE in gfc_trans_array_constructor, at fortran/trans-array.c...
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: fortran
4.3.0
: P3 normal
: 4.3.0
Assigned To: Paul Thomas
: ice-on-valid-code, patch
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2007-07-08 22:40 UTC by Janus Weil
Modified: 2007-07-29 14:45 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.1.3 4.2.1
Known to fail: 4.3.0
Last reconfirmed: 2007-07-09 22:40:38


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Janus Weil 2007-07-08 22:40:55 UTC
consider the following program:


program matrix

implicit none
real,dimension(2,2),parameter::c=0
real,dimension(2,2)::m

m=f()+c
m=c+f()
call sub(m+f())
call sub(c+m)
call sub(f()+c)
call sub(c+f())

contains

  function f()    
    implicit none
    real, dimension(2,2)::f
    f=0
  end function f

  subroutine sub(a)
    implicit none
    real, dimension(2,2)::a
  end subroutine sub

end program matrix


this gives the error message:

matrix.f90: In function ‘MAIN__’:
matrix.f90:11: internal compiler error: in gfc_trans_array_constructor, at fortran/trans-array.c:1664

though the message claims an error in line 11, the program only fails in the presence of line 12:
"call sub(c+f())"
all the lines before (7-11) are ok.

the ICE only appears in exactly this configuration, i.e. c is a parameter, f is a function, and c+f() is fed as an argument to a subroutine

it happens in trans-array.c(gfc_trans_array_constructor), line 1664:
/* We should have a 1-dimensional, zero-based loop.  */
gcc_assert (loop->dimen == 1);

this fails with loop->dimen==2 in our case

happens only with 4.3 (trunk), but not with 4.1 or 4.2
Comment 1 tobias.burnus 2007-07-09 08:15:16 UTC
Note: The program fails with
  "call sub(c+f())"
where c is a parameter and dimensions of c and f are (2,2). If one has dimension(2) it works.

Working with 2007-05-08-r124539, failing with 2007-05-09-r124566.
The follwing has been checked in during that time:

r124550 | pault | 2007-05-08 16:40:58 +0200 (Tue, 08 May 2007) | 7 lines
        PR 31630
        * resolve.c (resolve_symbol): Remove the flagging mechanism from the...
r124546 | pault | 2007-05-08 14:45:31 +0200 (Tue, 08 May 2007) | 11 lines
        PR 31692
        * trans-array.c (gfc_conv_array_parameter): Convert full array
        references to the result of the procedure enclusing the call.
r124541 | pault | 2007-05-08 13:58:25 +0200 (Tue, 08 May 2007) | 18 lines
        PR 29397, PR 29400
        * decl.c (add_init_expr_to_sym): Expand a scalar initializer
        for a parameter array into an array expression with the right
        shape.
        * array.c (spec_dimen_size): Remove static attribute.
        * gfortran.h : Prototype for spec_dimen_size.
   http://gcc.gnu.org/ml/gcc-patches/2007-05/msg00406.html

I think the last-shown commit caused the regression.
Comment 2 Janus Weil 2007-07-09 19:17:45 UTC
I checked it: The ICE is really introduced by rev. 124541.

> r124541 | pault | 2007-05-08 13:58:25 +0200 (Tue, 08 May 2007) | 18 lines
>         PR 29397, PR 29400
>         * decl.c (add_init_expr_to_sym): Expand a scalar initializer
>         for a parameter array into an array expression with the right
>         shape.
>         * array.c (spec_dimen_size): Remove static attribute.
>         * gfortran.h : Prototype for spec_dimen_size.
>    http://gcc.gnu.org/ml/gcc-patches/2007-05/msg00406.html
Comment 3 Paul Thomas 2007-07-09 22:40:38 UTC
(In reply to comment #2)

Whilst I agree that this is a regession, it is so because an underlying bug is exposed - in other words, r124541 is perfectly correct.

The following also fails:

program matrix

implicit none
real,dimension(2,2),parameter::c=reshape((/1,2,3,4/),(/2,2/))
real,dimension(2,2)::m

m=f()+c
m=c+f()
call sub(m+f())
call sub(c+m)
call sub(f()+c)
call sub(c+f())

contains

  function f()    
    implicit none
    real, dimension(2,2)::f
    f=0
  end function f

  subroutine sub(a)
    implicit none
    real, dimension(2,2)::a
    print *, a
  end subroutine sub

end program matrix

which was not affected by the above patch.

This fixes both versions of the bug:

Index: gcc/fortran/trans-array.c
===================================================================
*** gcc/fortran/trans-array.c   (revision 126461)
--- gcc/fortran/trans-array.c   (working copy)
*************** gfc_trans_array_constructor (gfc_loopinf
*** 1656,1661 ****
--- 1656,1673 ----

    /* See if the constructor determines the loop bounds.  */
    dynamic = false;
+
+   if (ss->expr->shape && loop->to[0] == NULL_TREE)
+     {
+       int n;
+       for (n = 0; n < ss->expr->rank; n++)
+       {
+         loop->to[n] = gfc_conv_mpz_to_tree (ss->expr->shape [n],
+                                             gfc_index_integer_kind);
+         loop->from[n] = gfc_index_zero_node;
+       }
+     }
+
    if (loop->to[0] == NULL_TREE)
      {
        mpz_t size;

Paul

Comment 4 Janus Weil 2007-07-11 21:39:02 UTC
Paul,
I tested your patch, and indeed it does fix the problem for me. I also checked it for regressions, and it doesn't seem to break anthing.
Cheers,
Janus
Comment 5 Daniel Franke 2007-07-24 13:45:51 UTC
Paul, any plans to get the fix from comment #3 into trunk?
Comment 6 Paul Thomas 2007-07-27 15:09:08 UTC
(In reply to comment #5)
> Paul, any plans to get the fix from comment #3 into trunk?
> 

Gee, Daniel - I had totally forgotten this one.  Will do.

Paul
Comment 7 Francois-Xavier Coudert 2007-07-28 20:47:02 UTC
(In reply to comment #3)
> Index: gcc/fortran/trans-array.c
> ===================================================================
> *** gcc/fortran/trans-array.c   (revision 126461)
> --- gcc/fortran/trans-array.c   (working copy)
> *************** gfc_trans_array_constructor (gfc_loopinf
> *** 1656,1661 ****
> --- 1656,1673 ----
> 
>     /* See if the constructor determines the loop bounds.  */
>     dynamic = false;
> +
> +   if (ss->expr->shape && loop->to[0] == NULL_TREE)
> +     {
> +       int n;
> +       for (n = 0; n < ss->expr->rank; n++)
> +       {
> +         loop->to[n] = gfc_conv_mpz_to_tree (ss->expr->shape [n],
> +                                             gfc_index_integer_kind);
> +         loop->from[n] = gfc_index_zero_node;
> +       }
> +     }
> +
>     if (loop->to[0] == NULL_TREE)
>       {
>         mpz_t size;

This is OK to commit.
Comment 8 Paul Thomas 2007-07-29 11:21:15 UTC
(In reply to comment #7)
> (In reply to comment #3)

> This is OK to commit.
> 

FX,

In developing the testcase, I found out that this version of the patch is wrong - change 'f=0' to 'f=42' to see what I mean (look at the last line).

The loop should be unity based and then it works fine.

I will commit after regtesting.

Thanks

Paul
Comment 9 Paul Thomas 2007-07-29 14:44:20 UTC
Subject: Bug 32682

Author: pault
Date: Sun Jul 29 14:44:03 2007
New Revision: 127044

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127044
Log:
2007-07-29  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/31211
	* trans-expr.c (gfc_conv_expr_reference): Add block for case of
	scalar pointer functions so that NULL result is correctly
	handled.

	PR fortran/32682
	*trans-array.c (gfc_trans_array_constructor): On detecting a
	multi-dimensional parameter array, set the loop limits.

2007-07-29  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/31211
	* gfortran.dg/actual_pointer_function_1.f90: New test.

	PR fortran/32682
	* gfortran.dg/scalarize_parameter_array_1.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90
    trunk/gcc/testsuite/gfortran.dg/scalarize_parameter_array_1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog

Comment 10 Paul Thomas 2007-07-29 14:45:09 UTC
Fixed on trunk with correction to be unity rather than zero based.

Paul