[Bug fortran/43829] New: Scalarization of reductions

[Bug fortran/43829] New: Scalarization of reductions

[rguenth at gcc dot gnu dot org]

465.tonto in one of its hot loops does essentially what the following reduced
testcase does:

subroutine make_esss(esss,Ix,Iyz,e_x,ii_ivec)
  real(kind=kind(1.0d0)), dimension(:), intent(inout) :: esss
  real(kind=kind(1.0d0)), dimension(:,:), pointer :: Ix,Iyz
  integer(kind=kind(1)), dimension(:), pointer  :: e_x,ii_ivec

  esss(:) = esss(:) + sum(Ix(:,e_x) * Iyz(:,ii_ivec), 1)

end subroutine

this is scalarized by the frontend to

  atmp4 = e_x
  atmp6 = ii_ivec
  atmp8 = Ix(:,atmp4) * Iyz(:,atmp6)
  atmp11 = sum (atmp8, 1)
  ess = ess + atmp11

where the sum is not inline-expanded.

1) the temporaries for e_x and ii_ivec are not necessary
2) the sum can easily be inline-expanded as the shape of atmp8 is well-defined
3) we can avoid atmp8 by expanding sum(Ix(:,e_x), Iyz(:ii_ivec), 1) together
   like

                atmp11(z) = 0
                do z=1,size(Ix,1)
                  atmp11(z) = atmp11(z) + Ix(z,e_x(e)) * Iyz(z,ii_ivec(e))
                end do

   or even avoid atmp11 alltogether and expand to

              do e=1,size(esss,1)
                tem = 0
                do z=1,size(Ix,1)
                  tem = tem + Ix(z,e_x(e)) * Iyz(z,ii_ivec(e))
                end do
                esss(e) = esss(e) + tem
              end do

   given that esss does not have the target attribute and thus cannot
   be aliased by e_x or ii_ivec.


-- 
           Summary: Scalarization of reductions
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: enhancement
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions with Fortran array expressions

[steven at gcc dot gnu dot org]

------- Comment #1 from steven at gcc dot gnu dot org  2010-04-21 12:51 -------
Scalarization is just difficult...


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pault at gcc dot gnu dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-04-21 12:51:53
               date|                            |
            Summary|Scalarization of reductions |Scalarization of reductions
                   |                            |with Fortran array
                   |                            |expressions


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2010-04-21 13:08 -------
Wade-through-the-code dump:

gfc_trans_assignment_1 gets

  make_esss:esss(:)
  (+ make_esss:esss(:) _gfortran_sum_r8[[(((* make_esss:ix(: ,
__convert_i4_i8[[((make_esss:e_x(FULL)))]]) make_esss:iyz(: ,
__convert_i4_i8[[((make_esss:ii_ivec(FULL)))]]))) (_8) ((arg not-present)))]])

the first issue is that while we can properly scalarize

subroutine test0(esss,Ix,Iyz)
  real(kind=kind(1.0d0)), intent(out) :: esss
  real(kind=kind(1.0d0)), dimension(:) :: Ix,Iyz
  esss = sum(Ix * Iyz, 0)
end subroutine

we can't handle

subroutine test1(esss,Ix,Iyz)
  real(kind=kind(1.0d0)), dimension(:), intent(out) :: esss
  real(kind=kind(1.0d0)), dimension(:,:) :: Ix,Iyz
  esss = sum(Ix * Iyz, 0)
end subroutine

as gfc_conv_intrinsic_function does not handle expr->rank > 0 for
inline expansion.

The second issue is that for

subroutine test0(esss,Ix,Iyz, e_x, ii_ivec)
  real(kind=kind(1.0d0)), dimension(:), intent(out) :: esss
  real(kind=kind(1.0d0)), dimension(:) :: Ix,Iyz
  integer(kind=kind(1)), dimension(:) :: e_x,ii_ivec
  esss = Ix(e_x) * Iyz(ii_ivec)
end subroutine

we create temporaries for e_x and ii_ivec.  Even for

subroutine test0(esss,Ix, e_x)
  real(kind=kind(1.0d0)), dimension(:), intent(out) :: esss
  real(kind=kind(1.0d0)), dimension(:) :: Ix
  integer(kind=kind(1)), dimension(:) :: e_x
  esss = Ix(e_x)
end subroutine


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Scalarization of reductions |Scalarization of reductions
                   |with Fortran array          |
                   |expressions                 |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2010-04-21 14:26 -------
subroutine test0(esss,Ix, e_x)
  real(kind=kind(1.0d0)), dimension(:), intent(out) :: esss
  real(kind=kind(1.0d0)), dimension(:) :: Ix
  integer(kind=kind(1)), dimension(:) :: e_x
  esss = Ix(e_x)
end subroutine

where we create the temporary during gfc_conv_loop_setup ->
gfc_add_loop_ss_code
when handling the GFC_SS_SECTION subscript e_x and want to get its array
descriptor.

To be able to specialize this we'd need to know whether at the end we're
going to need a descriptor for the GFC_SS_SECTION (because we are passing
it to a function or so) or whether we are going to access single elements only.

It looks like this is all because of the convert intrinsic called by
__convert_i4_i8[[((test0:e_x(FULL)))]] which is emitted regardless
of the type of e_x (even if it is integer(kind=kind(8))).

Testcase is fixed by

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c       (revision 158590)
+++ gcc/fortran/resolve.c       (working copy)
@@ -4007,8 +4007,7 @@ gfc_resolve_index (gfc_expr *index, int
                        &index->where) == FAILURE)
       return FAILURE;

-  if (index->ts.kind != gfc_index_integer_kind
-      || index->ts.type != BT_INTEGER)
+  if (index->ts.type != BT_INTEGER)
     {
       gfc_clear_ts (&ts);
       ts.type = BT_INTEGER;
Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c   (revision 158590)
+++ gcc/fortran/trans-array.c   (working copy)
@@ -2434,6 +2434,7 @@ gfc_conv_array_index_offset (gfc_se * se
                                          gfc_conv_array_data (desc));
          index = gfc_build_array_ref (data, index, NULL);
          index = gfc_evaluate_now (index, &se->pre);
+         index = fold_convert (gfc_array_index_type, index);

          /* Do any bounds checking on the final info->descriptor index.  */
          index = gfc_trans_array_bound_check (se, info->descriptor,


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2010-04-22 08:34 -------
Subject: Bug 43829

Author: rguenth
Date: Thu Apr 22 08:34:41 2010
New Revision: 158632

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158632
Log:
2010-04-22  Richard Guenther  <rguenther@suse.de>

        PR fortran/43829
        * resolve.c (gfc_resolve_index): Wrap around ...
        (gfc_resolve_index_1): ... this.  Add parameter to allow
        any integer kind index type.
        (resolve_array_ref): Allow any integer kind for the start
        index of an array ref.

        * gfortran.dg/vector_subscript_6.f90: New testcase.
        * gfortran.dg/assign_10.f90: Adjust.

Added:
    trunk/gcc/testsuite/gfortran.dg/vector_subscript_6.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/assign_10.f90


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2010-04-22 14:00 -------
Now onto

  subroutine test1(esss,Ix,Iyz)
    real(kind=kind(1.0d0)), dimension(:), intent(out) :: esss
    real(kind=kind(1.0d0)), dimension(:,:) :: Ix,Iyz
    esss = sum(Ix * Iyz, 1)
  end subroutine

noting that we can exchange the sum and the indexing of the assignment
making the sum() of rank one (and thus eligible for inline expansion).

Thus, expand it as

  do i=1,size(esss)
    esss(i) = sum(Ix(i,:) * Iyz(i,:))
  end do


An even simpler testcase where we want to do this exchange is

  subroutine test0(esss,Ix,Iyz)
    real(kind=kind(1.0d0)), dimension(:), intent(out) :: esss
    real(kind=kind(1.0d0)), dimension(:,:), intent(in) :: Ix
    esss = sum(Ix, 1)
  end subroutine

Thus, whenever the reduction is one-dimensional (though in this simpler
testcase we do not avoid the temporary for the sum intrinsic argument
which was the point of the excercise).  So, slightly more complicated
to show that this is still beneficial:

  subroutine test0(esss,Ix,Iyz)
    real(kind=kind(1.0d0)), dimension(:), intent(out) :: esss
    real(kind=kind(1.0d0)), dimension(:,:), intent(in) :: Ix
    esss = esss + sum(Ix, 1)
  end subroutine


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2010-04-22 15:39 -------
Reduced testcase:

template <class T> void void_cast_register(T *) __attribute__ ((used));
template <class T> void void_cast_register(T *) { }


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2010-04-23 16:09 -------
Mine.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2010-04-21 12:51:53         |2010-04-23 16:09:17
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #8 from mikael at gcc dot gnu dot org  2010-05-04 23:08 -------
Created an attachment (id=20558)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20558&action=view)
draft patch

This uses the scalarizer to inline. 
The gfc_loopinfo is modified twice, first after walking args for passing
through the scalarizer with the same shape as other arrays, and then before the
expression evaluation (it is restored). 

There is still a problem with vector subscript but other tests should work
fine. 
And there are probably regressions.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[pault at gcc dot gnu dot org]

------- Comment #9 from pault at gcc dot gnu dot org  2010-05-05 05:07 -------
(In reply to comment #8)
> Created an attachment (id=20558)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20558&action=view) [edit]
> draft patch

Mikael,

I am pretty much out of the loop for the next two weeks because I am on trips
for my day-time work.  In order to rush this along, you might have to walk one
of the others through your patch or to describe its core function here.

As far as I can see from a quick look, most of the patch is consequent on the
change to gfc_ss_info and that most of the meat comes at the end.  I have not
yet studied that in detail but it looks as if it is pretty easily explained.

Anyway, welcome back!

Paul


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #10 from rguenth at gcc dot gnu dot org  2010-05-05 15:44 -------
Indeed the patch looks a lot nicer and less like a hack than mine.

Re-assigning to you ;)


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|rguenth at gcc dot gnu dot  |mikael at gcc dot gnu dot
                   |org                         |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #11 from mikael at gcc dot gnu dot org  2010-05-22 19:36 -------
Created an attachment (id=20726)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20726&action=view)
updated patch

Update on this :
The attached patch seems to work on sum. 
I plan to reorganize some structs, but the behavior will remain the same. 

Regression fixes apart, what's different in this patch, is that the scalarizer
is aware of nested loops (in the previous patch, loops were removed/added at
appropriate places to fit the scalarizer's expectations). 

The patch also handles inlining for transpose because inlining was an easy fix
to a regression there was. 


However, regressions are now on transpose :
 - alloc_comp_transformational_1.f90 : double free on function exit. I will
have to investigate.
 - char_length_8.f90 : reshape doesn't like being passed a transposed array as
target argument because the shape doesn't match then. I will look if i can fix
reshape without recursing into some other regression.
 - transpose_2.f90 : I will have to see if it is a real regression or if only
the error message has changed. 

There is also this strange one.
 - pr43984.f90 : memory allocation error at compile time (???!!)


-- 

mikael at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20558|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #12 from mikael at gcc dot gnu dot org  2010-06-25 09:27 -------
(In reply to comment #11)
> However, regressions are now on transpose :
>  - alloc_comp_transformational_1.f90 : double free on function exit. I will
> have to investigate.
Fixed

>  - char_length_8.f90 : reshape doesn't like being passed a transposed array as
> target argument because the shape doesn't match then. I will look if i can fix
> reshape without recursing into some other regression.
Fixed.

>  - transpose_2.f90 : I will have to see if it is a real regression or if only
> the error message has changed. 
The error message has changed, no real regression. 

> 
> There is also this strange one.
>  - pr43984.f90 : memory allocation error at compile time (???!!)
It was a double free, fixed.  


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #13 from mikael at gcc dot gnu dot org  2010-06-25 09:34 -------
Created an attachment (id=21002)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21002&action=view)
Updated patch

As I was having memory corruption regressions, I started to run valgrind on
some testcases, whose fixes brought new memory problems, and so on.
Now the patch contains a non negligible part of completely unrelated memory
fixes, and also random other things (dead code removal, if-blocks turned into
asserts, ...).


-- 

mikael at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20726|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #14 from mikael at gcc dot gnu dot org  2010-06-25 09:36 -------
TODO:
 * testcases to check anything that might be impacted by the patch
 * split the patch for review & commit


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #15 from mikael at gcc dot gnu dot org  2010-06-25 09:42 -------
Isn't there a problem with the size of the patch ?
pr43829_3.diff  46.78 KB

I get here:
% du -h pr43829_3.diff  
240K    pr43829_3.diff
%

At least it is not truncated (I have just checked).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenther at suse dot de]

------- Comment #16 from rguenther at suse dot de  2010-06-25 09:45 -------
Subject: Re:  Scalarization of reductions

On Fri, 25 Jun 2010, mikael at gcc dot gnu dot org wrote:

> ------- Comment #15 from mikael at gcc dot gnu dot org  2010-06-25 09:42 -------
> Isn't there a problem with the size of the patch ?
> pr43829_3.diff  46.78 KB
> 
> I get here:
> % du -h pr43829_3.diff  
> 240K    pr43829_3.diff
> %
> 
> At least it is not truncated (I have just checked).

Downloading it shows

/tmp> du -h attachment.cgi?id=21002
220K    attachment.cgi?id=21002

so I guess bugzilla reports compressed size (it stores compresses 
plaintext)

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[dominiq at lps dot ens dot fr]

------- Comment #17 from dominiq at lps dot ens dot fr  2010-06-25 22:08 -------
With the patch in comment #13 I get the following failures:


FAIL: gfortran.dg/entry_10.f90  -O0  (internal compiler error)
...
FAIL: gfortran.dg/entry_13.f90  -O0  (internal compiler error)
...
FAIL: gfortran.dg/entry_16.f90  -O0  (internal compiler error)
...

FAIL: gfortran.dg/transpose_2.f90  -O0  output pattern test, is At line 16 of
file /opt/gcc/work/gcc/testsuite/gfortran.dg/transpose_2.f90

The reason for the failures with entry_* is

[macbook] f90/bug% gfc /opt/gcc/_clean/gcc/testsuite/gfortran.dg/entry_10.f90
f951(52154) malloc: *** error for object 0x1419161b0: pointer being freed was
not allocated
*** set a breakpoint in malloc_error_break to debug
f951: internal compiler error: Abort trap

For the transpose test the runtime error is

At line 16 of file /opt/gcc/_clean/gcc/testsuite/gfortran.dg/transpose_2.f90
Fortran runtime error: Array bound mismatch for dimension 1 of array 'b' (3/2)

instead of 

Fortran runtime error: Incorrect extent in return value of TRANSPOSE intrinsic
in dimension 1: is 2, should be 3

I have also a dozen ICEs in my favorite can of worms, for instance with


module grid_module
  implicit none 
  type grid
  end type
  type field
    type(grid) :: mesh
  end type
contains
  real function return_x(this)
    class(grid) :: this
  end function
end module 

module field_module
  use grid_module, only: field,return_x
  implicit none 
contains
  subroutine output(this)
    class(field) :: this
    print *,return_x(this%mesh)
  end subroutine
end module


[macbook] f90/bug% gfc pr42051_3.f90
pr42051_3.f90:21:0: internal compiler error: Segmentation fault
Please submit a full bug report,


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #18 from mikael at gcc dot gnu dot org  2010-06-26 09:06 -------
(In reply to comment #17)
> The reason for the failures with entry_* is
> 
> [macbook] f90/bug% gfc /opt/gcc/_clean/gcc/testsuite/gfortran.dg/entry_10.f90
> f951(52154) malloc: *** error for object 0x1419161b0: pointer being freed was
> not allocated
> *** set a breakpoint in malloc_error_break to debug
> f951: internal compiler error: Abort trap
I will look into these. 

> 
> For the transpose test the runtime error is
> 
> At line 16 of file /opt/gcc/_clean/gcc/testsuite/gfortran.dg/transpose_2.f90
> Fortran runtime error: Array bound mismatch for dimension 1 of array 'b' (3/2)
> 
> instead of 
> 
> Fortran runtime error: Incorrect extent in return value of TRANSPOSE intrinsic
> in dimension 1: is 2, should be 3
I get these too, but the difference is just because the shape mismatch is now
caught by the scalarizer instead of the libgfortran.

> 
> I have also a dozen ICEs in my favorite can of worms, for instance with
> 
> 
> module grid_module
>   implicit none 
>   type grid
>   end type
>   type field
>     type(grid) :: mesh
>   end type
> contains
>   real function return_x(this)
>     class(grid) :: this
>   end function
> end module 
> 
> module field_module
>   use grid_module, only: field,return_x
>   implicit none 
> contains
>   subroutine output(this)
>     class(field) :: this
>     print *,return_x(this%mesh)
>   end subroutine
> end module
> 
> 
> [macbook] f90/bug% gfc pr42051_3.f90
> pr42051_3.f90:21:0: internal compiler error: Segmentation fault
> Please submit a full bug report,
> 
I will have a look too.


Thanks for testing.
There are probably some other regressions still uncaught. 
I will have to do a full valgrind-testing-enabled testsuite run at some point.
:-(


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #19 from mikael at gcc dot gnu dot org  2010-06-27 12:42 -------
Created an attachment (id=21016)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21016&action=view)
Updated patch


-- 

mikael at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #21002|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #20 from mikael at gcc dot gnu dot org  2010-06-27 12:50 -------
(In reply to comment #17)
> With the patch in comment #13 I get the following failures:
> 
> 
> FAIL: gfortran.dg/entry_10.f90  -O0  (internal compiler error)
> ...
> FAIL: gfortran.dg/entry_13.f90  -O0  (internal compiler error)
> ...
> FAIL: gfortran.dg/entry_16.f90  -O0  (internal compiler error)
Fixed. 

For info the relevant hunk is :
diff --git a/decl.c b/decl.c
index aa7a266..d062433 100644
--- a/decl.c
+++ b/decl.c
@@ -840,10 +850,10 @@ get_proc_name (const char *name, gfc_symbol **result,
bool module_fcn_entry)
             can be applied.  */
          (*result)->ns = gfc_current_ns;

-         gfc_find_sym_tree (name, gfc_current_ns, 0, &st);
-         st->n.sym = *result;
          st = gfc_get_unique_symtree (gfc_current_ns);
-         st->n.sym = sym;
+         gfc_assign_symbol (&st->n.sym, sym);
+         gfc_find_sym_tree (name, gfc_current_ns, 0, &st);
+         gfc_assign_symbol (&st->n.sym, *result);
        }
     }
   else

i.e. the two symbol assignments are exchanged so that the symbol is not deleted
between them. 



> 
> I have also a dozen ICEs in my favorite can of worms, for instance with
> 
> 
> module grid_module
>   implicit none 
>   type grid
>   end type
>   type field
>     type(grid) :: mesh
>   end type
> contains
>   real function return_x(this)
>     class(grid) :: this
>   end function
> end module 
> 
> module field_module
>   use grid_module, only: field,return_x
>   implicit none 
> contains
>   subroutine output(this)
>     class(field) :: this
>     print *,return_x(this%mesh)
>   end subroutine
> end module
> 
> 
There was a missing "end" statement to make it fail.

Fixed, for info the relevant part is the change in gfc_find_symbol_vtab,
espcially this hunk: 

diff --git a/class.c b/class.c
index 37b9cf0..11af809 100644
--- a/class.c
+++ b/class.c
@@ -641,7 +730,17 @@ gfc_find_derived_vtab (gfc_symbol *derived, bool resolved)
     add_generics_to_declared_vtab (derived, vtab->ts.u.derived,
                                   derived, resolved);

-  return vtab;
+  found_sym = vtab;
+
+cleanup:
+  /* It is unexpected to have some symbols added at resolution or code
+     generation time. We commit the changes in order to keep a clean state. 
*/
+  if (found_sym)
+    gfc_commit_symbols ();
+  else
+    gfc_undo_symbols ();
+
+  return found_sym;
 }




-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[dominiq at lps dot ens dot fr]

------- Comment #21 from dominiq at lps dot ens dot fr  2010-06-27 19:20 -------
With the patch in comment #19, the test suite pass with -m32 and -m64, but for
gfortran.dg/transpose_2.f90 which needs an adjustment of the dg-error.

AFAICT the SUM of the different tests are scalarized (it would be interesting
to have some timings before and after the patch for 465.tonto).

The ICEs for the tests in pr44660 are now replaced with errors:

Error: Unexpected EQUIVALENCE statement at (1)

(although I am puzzled by the following error for the second test:


pr44660_1.f90:2.21:

         IF(DBUG.AND.NX.GT.0) THEN
                     1
Error: Operands of logical operator '.and.' at (1) are REAL(4)/LOGICAL(4)

where NX is an INTEGER(4) rather than a LOGICAL(4)).

The test in pr39304 ICE with

pr39304.f90: In function 'sum_k1a':
pr39304.f90:2626:0: internal compiler error: in conv_intrinsic_arith, at
fortran/trans-intrinsic.c:1875

instead of

pr39304.f90: In function 'sd_matrix_one':
pr39304.f90:440:0: internal compiler error: Segmentation fault

I see several ICEs:

f951: internal compiler error: in match_procedure_in_type, at
fortran/decl.c:7809

in the tests from pr41951, pr43199, and pr43896. Note that the later compiled
with trunk does not link with:

Undefined symbols:
  "_vtab$t_rotation_matrix.2314", referenced from:
      ___m_vector_MOD_rotation_matrix_times_vector in cc80aib8.o
...

and pr41951.f90 ICE with

f951: internal compiler error: in gfc_match_varspec, at fortran/primary.c:1821

I am probably missing other differences, but this represents most of them.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[burnus at gcc dot gnu dot org]

------- Comment #22 from burnus at gcc dot gnu dot org  2010-06-27 19:44 -------
(In reply to comment #21)
> (although I am puzzled by the following error for the second test:
>          IF(DBUG.AND.NX.GT.0) THEN
>                      1
> Error: Operands of logical operator '.and.' at (1) are REAL(4)/LOGICAL(4)
> where NX is an INTEGER(4) rather than a LOGICAL(4)).

The error seems to be OK: Seemingly "DBUG" is REAL(4) and the expression (NX <
0) is a default-kind logical.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[dominiq at lps dot ens dot fr]

------- Comment #23 from dominiq at lps dot ens dot fr  2010-06-27 20:05 -------
(In reply to comment #22)
> > (although I am puzzled by the following error for the second test:
> >          IF(DBUG.AND.NX.GT.0) THEN
> >                      1
> > Error: Operands of logical operator '.and.' at (1) are REAL(4)/LOGICAL(4)
> > where NX is an INTEGER(4) rather than a LOGICAL(4)).
> 
> The error seems to be OK: Seemingly "DBUG" is REAL(4) and the expression (NX <
> 0) is a default-kind logical.

OK, I did not parse NX.GT.0.

Now I have forgotten another ICE with the patch: the second invalid test in
comment #2 gives

pr43829_2.f90: In function 'test1':
pr43829_2.f90:4:0: internal compiler error: in gfc_conv_section_startstride, at
fortran/trans-array.c:3114


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[dominiq at lps dot ens dot fr]

------- Comment #24 from dominiq at lps dot ens dot fr  2010-06-27 21:30 -------
(In reply to comment #23)
> Now I have forgotten another ICE with the patch: the second invalid test in
> comment #2 gives

See pr44693.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[paul dot richard dot thomas at gmail dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]



------- Comment #25 from paul dot richard dot thomas at gmail dot com  2010-06-28 04:27 -------
Subject: Re:  Scalarization of reductions

Dear Mikael,

> -         gfc_find_sym_tree (name, gfc_current_ns, 0, &st);
> -         st->n.sym = *result;
>          st = gfc_get_unique_symtree (gfc_current_ns);
> -         st->n.sym = sym;
> +         gfc_assign_symbol (&st->n.sym, sym);
> +         gfc_find_sym_tree (name, gfc_current_ns, 0, &st);
> +         gfc_assign_symbol (&st->n.sym, *result);
>        }
>     }
>   else
>
> i.e. the two symbol assignments are exchanged so that the symbol is not deleted
> between them.

...snip...

> -  return vtab;
> +  found_sym = vtab;
> +
> +cleanup:
> +  /* It is unexpected to have some symbols added at resolution or code
> +     generation time. We commit the changes in order to keep a clean state.
> */
> +  if (found_sym)
> +    gfc_commit_symbols ();
> +  else
> +    gfc_undo_symbols ();
> +
> +  return found_sym;
>  }

I have been round long enough that I should have remembered all of this :-(

Thanks!

Paul


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[rguenth at gcc dot gnu dot org]

------- Comment #26 from rguenth at gcc dot gnu dot org  2010-06-28 09:35 -------
(In reply to comment #21)
> With the patch in comment #19, the test suite pass with -m32 and -m64, but for
> gfortran.dg/transpose_2.f90 which needs an adjustment of the dg-error.
> 
> AFAICT the SUM of the different tests are scalarized (it would be interesting
> to have some timings before and after the patch for 465.tonto).

                                  Estimated                       Estimated
                Base     Base       Base        Peak     Peak       Peak
Benchmarks      Ref.   Run Time     Ratio       Ref.   Run Time     Ratio
-------------- ------  ---------  ---------    ------  ---------  ---------
465.tonto        9840        390       25.2 S    9840        361       27.3 S
465.tonto        9840        391       25.2 S    9840        361       27.3 *
465.tonto        9840        390       25.2 *    9840        362       27.2 S
==============================================================================
465.tonto        9840        390       25.2 *    9840        361       27.3 *

base is trunk r161367, peak is trunk r161367 + the patch from comment #19.
Flags are -O3 -ffast-math -funroll-loops -march=core2, executed on an
iCore7 @ 3.3GHz.

Thus your patch improves performance by 7.5% which is very nice and even
above what I expected (compared to manual source manipulation of the
hottest SUM).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[dominiq at lps dot ens dot fr]

------- Comment #27 from dominiq at lps dot ens dot fr  2010-09-08 11:29 -------
What is the fate of the patch in comment #19?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #28 from mikael at gcc dot gnu dot org  2010-09-08 17:21 -------
(In reply to comment #27)
> What is the fate of the patch in comment #19?
> 
Some (admittedly small) parts of the patch are already on trunk.
The transpose patches still waiting review at
http://gcc.gnu.org/ml/fortran/2010-09/msg00109.html are the first important
step to commit this patch (they prevent one regression).
The rest is decaying. :-(
But it can still be updated and committed before the end of stage 1. :-)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[dominiq at lps dot ens dot fr]

------- Comment #29 from dominiq at lps dot ens dot fr  2010-09-13 09:09 -------
> But it can still be updated and committed before the end of stage 1. :-)

I hope so!-) I also think this pr is related to pr43829.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829

[Bug fortran/43829] Scalarization of reductions

[mikael at gcc dot gnu dot org]

------- Comment #30 from mikael at gcc dot gnu dot org  2010-09-13 17:09 -------
(In reply to comment #29)
> I also think this pr is related to pr43829.
> 

It couldn't be more ;-)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829