Bug 37749 - ICE on array section with vector subscript
ICE on array section with vector subscript
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: fortran
4.4.0
: P3 normal
: ---
Assigned To: Not yet assigned to anyone
: ice-on-valid-code
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2008-10-06 14:02 UTC by Jakub Jelinek
Modified: 2008-11-01 19:47 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.1.3 4.2.2 4.3.0 4.4.0
Last reconfirmed: 2008-10-06 19:47:31


Attachments
proposed patch, regression tested (991 bytes, patch)
2008-10-29 21:06 UTC, Mikael Morin
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2008-10-06 14:02:52 UTC
subroutine subr (m, n, a, b, c, d, p)
  implicit none
  integer m, n
  real a(m,n), b(m,n), c(n,n), d(m,n)
  integer p(n)
  d = a(:,p) - matmul(b, c)
end subroutine

  implicit none
  integer i
  real a(3,2), b(3,2), c(2,2), d(3,2)
  integer p(2)
  a = reshape ((/(i, i = 1, 6)/), (/3, 2/))
  b = 1
  c = 2
  p = 2
  call subr (3, 2, a, b, c, d, p)
  if (any (d .ne. reshape ((/(mod (i + 2, 3), i = 1, 6)/), (/3, 2/)))) call abort
end

ICEs because a loop bound (loop->to[0]) hasn't been set.
Comment 1 Mikael Morin 2008-10-29 20:59:53 UTC
Reduced case:

subroutine subr (m, n, a, b, c, d, p)
  implicit none
  integer m, n
  real a(m,n), b(m,n), c(n,n), d(m,n)
  integer p(n)
  d = a(:,p) - matmul(b, c)
end subroutine


The problem is with a(:,p) - matmul(b,c)

It arises because:
(1) gfc_conv_loop_setup chooses matmul's ss to setup the loop bounds. 
    This explains why -matmul(b,c) + a(:,p) works.
(2) The loop->to are asserted to be NULL in gfc_conv_loop_setup
    (GFC_SS_FUNCTION case)
(3) gfc_add_ss_loop_code for the second dimension of a(:,p) calls
    gfc_set_loop_bounds_from_array_spec which sets the second dimension of
    the loop.to
(4) in the loop setting the descriptor in gfc_trans_create_temp_array, 
    in the first iteration, (n = 0), loop.to is NULL, and the size is set to
    NULL. 
(5) In the next iteration, (n = 1), loop.to != NULL, and the loop follows
    the normal path. The size however was set to NULL (condition (4)).


(4) and (5) explain why it works with a(q,:) - matmul(b,c)
Comment 2 Mikael Morin 2008-10-29 21:06:09 UTC
Created attachment 16585 [details]
proposed patch, regression tested

This patch solves the problem by going through all the loop dimensions in gfc_trans_create_temp_array to look for a NULL loop->to before the actual loop execution and set size to NULL if one is found (it solves conditions (4) and (5)).
Comment 3 Paul Thomas 2008-10-30 11:13:24 UTC
(In reply to comment #2)
> Created an attachment (id=16585) [edit]
> proposed patch, regression tested
> 
> This patch solves the problem by going through all the loop dimensions in
> gfc_trans_create_temp_array to look for a NULL loop->to before the actual loop
> execution and set size to NULL if one is found (it solves conditions (4) and
> (5)).
> 
Mikael,

I was going to suggest that you look at this PR, since you have clearly attacked  the scalarizer.  Same question as before - do you have commit rights?

Paul
Comment 4 Paul Thomas 2008-10-30 20:47:39 UTC
Subject: Bug 37749

Author: pault
Date: Thu Oct 30 20:45:09 2008
New Revision: 141467

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141467
Log:
2008-10-30  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37903
        * trans-array.c (gfc_trans_create_temp_array): If n is less
	than the temporary dimension, assert that loop->from is
	zero (reverts to earlier versions). If there is at least one
	null loop->to[n], it is a callee allocated array so set the
	size to NULL and break.
	(gfc_trans_constant_array_constructor): Set the offset to zero.
	(gfc_trans_array_constructor): Remove loop shifting around the
	temporary creation.
	(gfc_conv_loop_setup): Prefer zero-based descriptors if
	possible.  Calculate the translation from loop variables to
	array indices if an array constructor.

2008-10-30  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37749
        * trans-array.c (gfc_trans_create_temp_array): If size is NULL
	use the array bounds for loop->to.

2008-10-30  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37903
        * gfortran.dg/vector_subscript_4.f90: New test.

2008-10-30  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37749
        * gfortran.dg/vector_subscript__5.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/vector_subscript_4.f90
    trunk/gcc/testsuite/gfortran.dg/vector_subscript_5.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/testsuite/ChangeLog

Comment 5 Paul Thomas 2008-11-01 19:47:08 UTC
Subject: Bug 37749

Author: pault
Date: Sat Nov  1 19:45:41 2008
New Revision: 141521

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141521
Log:
2008-11-01  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37903
        * trans-array.c (gfc_trans_create_temp_array): If n is less
	than the temporary dimension, assert that loop->from is
	zero (reverts to earlier versions). If there is at least one
	null loop->to[n], it is a callee allocated array so set the
	size to NULL and break.
	(gfc_trans_constant_array_constructor): Set the offset to zero.
	(gfc_trans_array_constructor): Remove loop shifting around the
	temporary creation.
	(gfc_conv_loop_setup): Prefer zero-based descriptors if
	possible.  Calculate the translation from loop variables to
	array indices if an array constructor.

2008-11-01  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37749
        * trans-array.c (gfc_trans_create_temp_array): If size is NULL
	use the array bounds for loop->to.

2008-11-01  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37903
        * gfortran.dg/vector_subscript_4.f90: New test.

2008-11-01  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37749
        * gfortran.dg/vector_subscript__5.f90: New test.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/vector_subscript_4.f90
    branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/vector_subscript_5.f90
Modified:
    branches/gcc-4_3-branch/gcc/fortran/ChangeLog
    branches/gcc-4_3-branch/gcc/fortran/trans-array.c
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog

Comment 6 Paul Thomas 2008-11-01 19:47:49 UTC
Jakub,

Thanks for the report - fixed by Mikael on trunk and 4.3.

Paul