[Bug middle-end/44945] New: [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[Bug middle-end/44945] New: [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

At revision 162192, I get with -m32:

FAIL: gfortran.dg/char_array_structure_constructor.f90  -O3
-fomit-frame-pointer  execution test

The failure is a "Bus error" with -O2 and -O3, and requires
-fomit-frame-pointer. Also the failure goes away when  Revision 162148 was OK
and the test passes on powerpc-apple-darwin9 (r162194).

The backtrace is

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x000adddc in __gfortran_compare_string (len1=4, s1=0x0, len2=4, s2=0x1f40
"wxyzabcdefgh") at intrinsics/string_intrinsics_inc.c:93
93        res = memcmp (s1, s2, ((len1 < len2) ? len1 : len2) * sizeof
(CHARTYPE));
(gdb) bt
#0  0x000adddc in __gfortran_compare_string (len1=4, s1=0x0, len2=4, s2=0x1f40
"wxyzabcdefgh") at intrinsics/string_intrinsics_inc.c:93
#1  0x00001e31 in main ()


-- 
           Summary: [4.6 Regression] FAIL:
                    gfortran.dg/char_array_structure_constructor.f90
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: dominiq at lps dot ens dot fr
 GCC build triplet: x86_64-apple-darwin10
  GCC host triplet: x86_64-apple-darwin10
GCC target triplet: x86_64-apple-darwin10


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

[Bug middle-end/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.6.0


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

[Bug fortran/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #1 from burnus at gcc dot gnu dot org  2010-07-15 14:37 -------
> 0x000adddc in __gfortran_compare_string (len1=4, s1=0x0, len2=4, s2=0x1f40

"s1" should not be NULL but the value of "c%a". The question is why does this
fail now. Recently added were: PURE and the "fn spec" of "..R.R". However, that
looks ok. (With Rev. 162202 it works on x86-64 -m64.)


trans-decl.c has:

  /* String functions.  */
  gfor_fndecl_compare_string = gfc_build_library_function_decl_with_spec (
        get_identifier (PREFIX("compare_string")), "..R.R",
        integer_type_node, 4, gfc_charlen_type_node, pchar1_type_node,
        gfc_charlen_type_node, pchar1_type_node);
  DECL_PURE_P (gfor_fndecl_compare_string) = 1;


-- 


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

[Bug fortran/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #2 from dominiq at lps dot ens dot fr  2010-07-15 15:23 -------
Created an attachment (id=21213)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21213&action=view)
dump with -m32 -O2 -fomit-frame-pointer -fdump-tree-optimized


-- 


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

[Bug fortran/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #3 from dominiq at lps dot ens dot fr  2010-07-15 15:24 -------
Created an attachment (id=21214)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21214&action=view)
dump with -m64 -O2 -fomit-frame-pointer -fdump-tree-optimized


-- 


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

[Bug fortran/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #4 from dominiq at lps dot ens dot fr  2010-07-15 15:27 -------
Created an attachment (id=21215)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21215&action=view)
dump with -m64 -O2 -fomit-frame-pointer -fdump-tree-optimized


-- 


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

[Bug middle-end/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #5 from burnus at gcc dot gnu dot org  2010-07-15 16:02 -------
According to gcc-testresults this seems to affect only Darwin, cf.
http://gcc.gnu.org/ml/gcc-testresults/2010-07/msg01432.html

The optimized dumps also seem to be OK (or at there is no obvious difference).

In the span working Rev. 162148 to failing Rev. 162192, I see the following
commits which might be related:

a) _gfortran_compare_string is now PURE,
   http://gcc.gnu.org/viewcvs?view=revision&revision=162160

b) Two MEMREF related commits,
   http://gcc.gnu.org/viewcvs?view=revision&revision=162175
   http://gcc.gnu.org/viewcvs?view=revision&revision=162177

c) Some commits I have overlooked


-- 


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

[Bug middle-end/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #6 from dominiq at lps dot ens dot fr  2010-07-15 16:25 -------
> a) _gfortran_compare_string is now PURE,
>   http://gcc.gnu.org/viewcvs?view=revision&revision=162160

The test passes with the following patch:


--- ../_clean/gcc/fortran/trans-decl.c  2010-07-15 18:05:19.000000000 +0200
+++ gcc/fortran/trans-decl.c    2010-07-15 18:07:53.000000000 +0200
@@ -2412,7 +2412,7 @@ gfc_build_intrinsic_function_decls (void
        get_identifier (PREFIX("compare_string")), "..R.R",
        integer_type_node, 4, gfc_charlen_type_node, pchar1_type_node,
        gfc_charlen_type_node, pchar1_type_node);
-  DECL_PURE_P (gfor_fndecl_compare_string) = 1;
+  /* DECL_PURE_P (gfor_fndecl_compare_string) = 1; */


-- 


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

[Bug middle-end/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #7 from dominiq at lps dot ens dot fr  2010-07-15 18:55 -------
Created an attachment (id=21218)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21218&action=view)
assembly generated with  -S -m32 -O2 -fomit-frame-pointer -fverbose-asm

The original file has been modified to have only one test:

if (any (c%a /= "wxyz")) call abort ()


-- 


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

[Bug middle-end/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[jakub at gcc dot gnu dot org]

------- Comment #8 from jakub at gcc dot gnu dot org  2010-07-15 20:20 -------
Ah, so -fdump-tree-optimized dump doesn't reveal the problem, but
-fdump-tree-optimized-all actually does.  The problem is that each routine -
char_array_structure_constructor and alloc, use different decls for the same
global variable, in my case __global_MOD_cD.1531 and __global_MOD_cD.1555.
When alloc is inlined into char_array_structure_constructor, this means
the same global var is accessed through 2 different aliases, and then
scheduling actually happens to reorder the store of the malloc result stored to
one of these c variables' data field with the read from the other c variable's
data field.

The DECL_PURE_P change just uncovered this latent issue, wonder why it hasn't
been seen earlier.  Guess Fortran code isn't inlined too much, with the
exception of contained functions.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org


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

[Bug fortran/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2010-07-16 08:38 -------
Frontend issue then.  Does it work with -fwhole-file?


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |fortran


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

[Bug fortran/44945] [4.6 Regression] FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #10 from dominiq at lps dot ens dot fr  2010-07-16 08:51 -------
> Does it work with -fwhole-file?

Nope, but it works with -fcheck=all at -O2 and not at -O3.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #11 from burnus at gcc dot gnu dot org  2010-07-16 09:11 -------
Seemingly, we currently only "recycle" DECLs for procedure decls, but not for
module variable decls.

Looking at
gfortran -fdump-tree-optimized-all -O3 char_array_structure_constructor.f90

one finds (grep MOD_c):
  __global_MOD_cD.1586
  __global_MOD_cD.1606
However, if one removes the second "USE global" and uses the host-associated
variable, it works (i.e. there is only one "__global_MOD_cD").

Expected: The same DECL is used for host associated variables - and, at least
with -fwhole-file, the same DECL is used for the whole file.

I wonder whether the same problem also exists for COMMON symbols.

(Note: While the wrong-code only becomes apparent on x86_64-apple-darwin10, the
bug itself exists on all platforms. Thus, I have emptied the PR's target
field.)


-- 

burnus 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
  GCC build triplet|x86_64-apple-darwin10       |
   GCC host triplet|x86_64-apple-darwin10       |
 GCC target triplet|x86_64-apple-darwin10       |
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2010-07-16 09:11:44
               date|                            |
            Summary|[4.6 Regression] FAIL:      |[4.6 Regression] Wrong decl
                   |gfortran.dg/char_array_struc|for module vars / FAIL:
                   |ture_constructor.f90        |gfortran.dg/char_array_struc
                   |                            |ture_constructor.f90


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[jakub at gcc dot gnu dot org]

------- Comment #12 from jakub at gcc dot gnu dot org  2010-07-16 10:08 -------
As can be seen on:
module mm
contains
  function a ()
    integer :: a, b, c
    common /blk/ b, c
    b = 6
    c = 7
    a = 8
  end function
end module mm
function b ()
  integer :: b, d, e
  common /blk/ d, e
  b = d + e
end function
program qq
  use mm
  integer :: d, e
  common /blk/ d, e
  interface
    function b ()
      integer :: b
    end function b
  end interface
  if (a () .ne. 8) call abort
  if (b () .ne. (d + e)) call abort
end
with -fdump-tree-original-all, I believe common decls are fine:
grep blk a.f90.003t.original
  static integer(kind=4)D.8 bD.1555 [value-expr: blk_D.1554.bD.1552];
  static integer(kind=4)D.8 cD.1556 [value-expr: blk_D.1554.cD.1553];
  static integer(kind=4)D.8 dD.1562 [value-expr: blk_D.1554.dD.1560];
  static integer(kind=4)D.8 eD.1563 [value-expr: blk_D.1554.eD.1561];
  static integer(kind=4)D.8 dD.1569 [value-expr: blk_D.1554.dD.1567];
  static integer(kind=4)D.8 eD.1570 [value-expr: blk_D.1554.eD.1568];


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #13 from burnus at gcc dot gnu dot org  2010-07-16 12:08 -------
Patch for -fwhole-file (not regtested).

Paul, do you know why derived types where excluded?

Index: trans-decl.c
===================================================================
--- trans-decl.c        (revision 162255)
+++ trans-decl.c        (working copy)
@@ -1149,11 +1149,9 @@ gfc_get_symbol_decl (gfc_symbol * sym)
     return sym->backend_decl;

   /* If use associated and whole file compilation, use the module
-     declaration.  This is only needed for intrinsic types because
-     they are substituted for one another during optimization.  */
+     declaration.  */
   if (gfc_option.flag_whole_file
        && sym->attr.flavor == FL_VARIABLE
-       && sym->ts.type != BT_DERIVED
        && sym->attr.use_assoc
        && sym->module)
     {


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #14 from burnus at gcc dot gnu dot org  2010-07-16 12:08 -------
(In reply to comment #13)
> Paul, do you know why derived types where excluded?
s/where/were/ - Firefox needs a grammar checker ...


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[jakub at gcc dot gnu dot org]

------- Comment #15 from jakub at gcc dot gnu dot org  2010-07-16 12:32 -------
I wonder whether the middle-end will not mind different types (the "recycled"
decl will use backend_decl and its type from wherever it has been generated,
and the current function will likely have a different type).  Or are backend
types shared for module derived types?  Perhaps conversion between the two
types will be considered useless, still something that needs checking.

Plus there is -fno-whole-file (unless we want to make -fwhole-file default and
remove -fno-whole-file for 4.6).


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #16 from burnus at gcc dot gnu dot org  2010-07-16 14:21 -------
(In reply to comment #15)
> I wonder whether the middle-end will not mind different types (the "recycled"
> decl will use backend_decl and its type from wherever it has been generated,
> and the current function will likely have a different type).

I am not sure whether I understand your concern. This is about recycling the
decl of a module variable (sym->attr.flavor == FL_VARIABLE) which should have
the same type everywhere. Thus, from that side there should be no problem.

The only potential issue I see is:

use module
type(t) :: x
x = y  ! where "y" is a module variable of type "t"

where "x" has locally been declared - and thus might have a different type decl
than the module variable. I think one should check this, but I think that does
not make the current patch wrong.

> Plus there is -fno-whole-file (unless we want to make -fwhole-file default and
> remove -fno-whole-file for 4.6).

My idea is to change at some point during 4.6 to using -fwhole-file by default
- though I would not yet remove -fno-whole-file, yet. But the ultimate goal is
to remove -fno-whole-file and add some new flag (-fpermissive) for the few
cases where one wants to disable compilations errors due to fwhole-file
diagnostics.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[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: 642 bytes --]



------- Comment #17 from paul dot richard dot thomas at gmail dot com  2010-07-16 14:38 -------
Subject: Re:  [4.6 Regression] Wrong decl for module vars / 
        FAIL: gfortran.dg/char_array_structure_constructor.f90

Dear Tobias,

On Fri, Jul 16, 2010 at 2:08 PM, burnus at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #13 from burnus at gcc dot gnu dot org  2010-07-16 12:08 -------
> Patch for -fwhole-file (not regtested).
>
> Paul, do you know why derived types where excluded?

No - maybe because the TYPE_CANONICAL had not been set?

Paul


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #18 from burnus at gcc dot gnu dot org  2010-07-16 14:47 -------
Answer: With the patch, one gets:
  gfortran -fwhole-file gfortran.dg/default_initialization_4.f90
without the patch (and still -fwhole-file) it works.

The reason is that one does not "copy" over the backend_decl for the components
- which one might need to do recursively as the components might again be
derived types. Thus, it fails at trans-expr.c's gfc_conv_component_ref for:
gcc_assert (c->backend_decl).

Actually, I do not quite understand why BT_CLASS works - the BT_DERIVED check
of comment 13 should be also false for BT_CLASS, which means its backend_decl
should already be imported with -fwhole-file.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #19 from burnus at gcc dot gnu dot org  2010-07-23 20:13 -------
Subject: Bug 44945

Author: burnus
Date: Fri Jul 23 20:13:07 2010
New Revision: 162487

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162487
Log:
2010-07-23  Tobias Burnus  <burnus@net-b.de>

        PR fortran/44945
        * trans-decl.c (gfc_get_symbol_decl): Use module decl with
        -fwhole-file also for derived types.
        * trans-types.c (copy_dt_decls_ifequal): Remove static and
        rename to gfc_copy_dt_decls_ifequal.
        (gfc_get_derived_type): Update call.
        * trans-types.h (gfc_copy_dt_decls_ifequal): Add prototype.

2010-07-23  Tobias Burnus  <burnus@net-b.de>

        PR fortran/44945
        * gfortran.dg/char_array_structure_constructor.f90: Add
        -fwhole-file as dg-option as it otherwise fails on some
        systems.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-types.c
    trunk/gcc/fortran/trans-types.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/char_array_structure_constructor.f90


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #20 from burnus at gcc dot gnu dot org  2010-07-23 20:15 -------
FIXED on the trunk. Thanks for the report Dominique and thanks Jakub for the
analysis.

I leave the PR open until the patch has been backported to 4.5.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #21 from dominiq at lps dot ens dot fr  2010-07-24 05:38 -------
In my opinion revision 162487 is only a partial fix of the problem. If I split
a modified test case in two files:

[macbook] f90/bug% cat char_array_structure_constructor_db_1.f90
MODULE global
  TYPE :: dt
    CHARACTER(4) a
    CHARACTER(4) b(2)
  END TYPE
  TYPE (dt), DIMENSION(:), ALLOCATABLE, SAVE :: c
END MODULE global
[macbook] f90/bug% cat char_array_structure_constructor_db_2.f90
program char_array_structure_constructor
  USE global
  call alloc (2)
  if (any (c%a /= "wxyz")) call abort ()
contains
  SUBROUTINE alloc (n)
    USE global
    ALLOCATE (c(n), STAT=IALLOC_FLAG)
    DO i = 1,n
      c (i) = dt ("wxyz",(/"abcd","efgh"/))
    ENDDO
  end subroutine alloc
END program char_array_structure_constructor

and compile it with

[macbook] f90/bug% gfc -fwhole-file -m32 -O3 -fomit-frame-pointer
char_array_structure_constructor_db_1.f90
char_array_structure_constructor_db_2.f90

I still get

[macbook] f90/bug% a.out
Bus error

Note that it is not surprising since -fwhole-file operates at the file level.
However if the test is valid fortran, then it should not be miscompiled if
split in two files.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #22 from burnus at gcc dot gnu dot org  2010-07-24 06:52 -------
(In reply to comment #21)
> In my opinion revision 162487 is only a partial fix of the problem. If I split
> a modified test case in two files [...] I still get [...] Bus error

Does -flto help?

> Note that it is not surprising since -fwhole-file operates at the file level.

Well, the middle-end also operates on single-file level: Thus, there is no
reason to break.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens dot fr]

------- Comment #23 from dominiq at lps dot ens dot fr  2010-07-24 07:49 -------
(In reply to comment #22)
> > In my opinion revision 162487 is only a partial fix of the problem. If I split
> > a modified test case in two files [...] I still get [...] Bus error
>
> Does -flto help?

Yes:

[macbook] f90/bug% gfc -flto -fwhole-file -m32 -O3 -fomit-frame-pointer
char_array_structure_constructor_db_1.f90
char_array_structure_constructor_db_2.f90
[macbook] f90/bug% a.out 
[macbook] f90/bug% 

> > Note that it is not surprising since -fwhole-file operates at the file level.
>
> Well, the middle-end also operates on single-file level: Thus, there is no
> reason to break.

I am not sure to understand the comment. In my opinion, valid multifile fortran
should never be miscompiled (and should not require -lto to do so!-).


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[rguenther at suse dot de]

------- Comment #24 from rguenther at suse dot de  2010-07-24 13:43 -------
Subject: Re:  [4.6 Regression] Wrong decl for module vars
 / FAIL: gfortran.dg/char_array_structure_constructor.f90

On Sat, 24 Jul 2010, dominiq at lps dot ens dot fr wrote:

> 
> 
> ------- Comment #23 from dominiq at lps dot ens dot fr  2010-07-24 07:49 -------
> (In reply to comment #22)
> > > In my opinion revision 162487 is only a partial fix of the problem. If I split
> > > a modified test case in two files [...] I still get [...] Bus error
> >
> > Does -flto help?
> 
> Yes:
> 
> [macbook] f90/bug% gfc -flto -fwhole-file -m32 -O3 -fomit-frame-pointer
> char_array_structure_constructor_db_1.f90
> char_array_structure_constructor_db_2.f90
> [macbook] f90/bug% a.out 
> [macbook] f90/bug% 
> 
> > > Note that it is not surprising since -fwhole-file operates at the file level.
> >
> > Well, the middle-end also operates on single-file level: Thus, there is no
> > reason to break.
> 
> I am not sure to understand the comment. In my opinion, valid multifile fortran
> should never be miscompiled (and should not require -lto to do so!-).

Look at -fdump-tree-original-uid dumps and see if you find the
same decl with two different UIDs.  I bet you will.

Richard.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #25 from burnus at gcc dot gnu dot org  2010-07-24 18:14 -------
(In reply to comment #21)
> In my opinion revision 162487 is only a partial fix of the problem. If I split
> a modified test case in two files: [...] I still get [...] Bus error

(In reply to comment #24)
> Look at -fdump-tree-original-uid dumps and see if you find the
> same decl with two different UIDs.  I bet you will.

One does. The problem is that when the module is not available.

The variable "c" (of example in comment 21) created for the main program
"char_array_structure_constructor" but if one searches for the symbol (in
gfc_get_symbol_decl) via
          gsym =  gfc_find_gsymbol (gfc_gsym_root, sym->module);
the result is NULL as the only gsym which exists in gfc_gsym_root is the one
for "char_array_structure_constructor" as there is none for the module. One
should create a "fake" gsym for modules, if the module does not exist, and put
there the variables, functions, and type declarations - such that they can be
found if needed.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu dot org]

------- Comment #26 from burnus at gcc dot gnu dot org  2010-07-28 07:41 -------
I expect a similar problem for derived types from modules which are external -
in that case the place to modify would be gfc_get_derived_type -- cf. PR 45077.
Also here, the proper gsym will not be available if the MODULE is only
available via the .mod.


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[pault at gcc dot gnu dot org]

------- Comment #27 from pault at gcc dot gnu dot org  2010-08-17 09:42 -------
(In reply to comment #25)
> (In reply to comment #21)
> > In my opinion revision 162487 is only a partial fix of the problem. If I split
> > a modified test case in two files: [...] I still get [...] Bus error
> 
> (In reply to comment #24)
> > Look at -fdump-tree-original-uid dumps and see if you find the
> > same decl with two different UIDs.  I bet you will.
> 
> One does. The problem is that when the module is not available.
> 
> The variable "c" (of example in comment 21) created for the main program
> "char_array_structure_constructor" but if one searches for the symbol (in
> gfc_get_symbol_decl) via
>           gsym =  gfc_find_gsymbol (gfc_gsym_root, sym->module);
> the result is NULL as the only gsym which exists in gfc_gsym_root is the one
> for "char_array_structure_constructor" as there is none for the module. One
> should create a "fake" gsym for modules, if the module does not exist, and put
> there the variables, functions, and type declarations - such that they can be
> found if needed.
> 

Tobias,

This is a very good suggestion. I will have a think about how to implement it
and will come back to you.  It is consistent with what I had in mind to improve
the efficiency of module reading.  ie. read_module proceeds as follows:

(i) On encountering a USE statement, look for the module gsym.  If it does not
exist, use read_module, as at present, to construct the namespace, without any
exclusions or renames;
(ii) In the current scope, create symtrees using the ONLY and the renames and
point to the symbols in the gsym namespace;
(iii) Subsequent USEs of the same module proceed by using the gsym namespace.

I think that Andy was smoking something or was having a whimsical spell when he
wrote module.c.  Whilst it is very elegant, it is unnecessarily disjunct from
the rest of the frontend and could have been done as above.

Cheers

Paul


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[mikael at gcc dot gnu dot org]

------- Comment #28 from mikael at gcc dot gnu dot org  2010-08-18 14:40 -------
(In reply to comment #27)
> This is a very good suggestion. I will have a think about how to implement it
> and will come back to you.  It is consistent with what I had in mind to improve
> the efficiency of module reading.  ie. read_module proceeds as follows:
> 
> (i) On encountering a USE statement, look for the module gsym.  If it does not
> exist, use read_module, as at present, to construct the namespace, without any
> exclusions or renames;
> (ii) In the current scope, create symtrees using the ONLY and the renames and
> point to the symbols in the gsym namespace;
> (iii) Subsequent USEs of the same module proceed by using the gsym namespace.

Hello, 

I have been trying a similar approach before.
One problem you may encounter is that symbols have non-shareable parts. 
For examples symbol attributes such as access or use_rename can differ between
symbols associated to the same entity.  
I tried to split gfc_symbol into an entity-specific part and a symbol-specific
part but that led to huge changes throughout the frontend and to the associated
regressions, so that I eventually gave up. 
A possible way might be to clone the symbol, and keep a pointer to the original
one so that we can get the original backend_decl from it. 

Maybe you will have a different path and won't encounter this problem at all. 
Good luck anyway, that seems the way to go :-)


-- 


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4


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

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[pault at gcc dot gnu.org]

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

Paul Thomas <pault at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot       |pault at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #29 from Paul Thomas <pault at gcc dot gnu.org> 2011-02-19 12:08:49 UTC ---
Created attachment 23403
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23403
A candidate fix for the PR

I believe that this does the job by generating the missing gsymbol and
populating its namespace with symbols to which backend_decls are attached.

As far as I can see, the same declaration is used consistently for c
(D.1737_69). 

Could somebody please check that I have hold of the right end of the stick?

Thanks

Paul

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[dominiq at lps dot ens.fr]

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

--- Comment #30 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-02-19 12:43:04 UTC ---
The bus error reported in comment #21 has disappeared from my scope between
revisions 167584 and 167726;-(

I'll test the patch in comment #29 ASAP.

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[burnus at gcc dot gnu.org]

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

--- Comment #31 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-02-19 13:08:26 UTC ---
(In reply to comment #29)
> A candidate fix for the PR

Aha, you have taken the simple route of modifying trans-decl.c and not the
whole module reading apparatus :-)

I think the patch is OK, though I would do some reordering, e.g. "sym->module"
is not needed as the whole block is enclosed in such a check.

I also would move the block down into
        if (gsym && gsym->ns && gsym->type == GSYM_MODULE)
which could be changed into
        if (!gsym || (gsym && gsym->ns && gsym->type == GSYM_MODULE))

And
-        gfc_find_symbol (sym->name, gsym->ns, 0, &s);
+         if (gsym)
+          gfc_find_symbol (sym->name, gsym->ns, 0, &s);

And then placing the "if (gsym == NULL)" in the
+       else if (s == NULL)
block.



(In reply to comment #30)
> The bus error reported in comment #21 has disappeared from my scope between
> revisions 167584 and 167726;-(

Ditto here, but looking at the dump, I see that the underlying issue is still
present - and it is fixed by the patch.

 * * *

I will do some more tests.

Related to those whole-file DECL issues: I wonder why there are still
differences between -flto and -fno-lto on Polyhedron (result is correct but
runtimes are different) and for Jack (-O3 miscompilation of xplor-nih's slink.f
and tenso.f, unless -fno-whole-file is used; -fno-inline-functions does not
help).

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[paul.richard.thomas at gmail dot com]

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

--- Comment #32 from paul.richard.thomas at gmail dot com <paul.richard.thomas at gmail dot com> 2011-02-19 15:07:03 UTC ---
Dear Tobias,


>
> Aha, you have taken the simple route of modifying trans-decl.c and not the
> whole module reading apparatus :-)

Of course - otherwise, we stand no chance of getting rid of the
regression.  It's only a small addition to the existing section of
code, so I thought that it had the advantage of being very clean too.

The revamp of module.c can wait until we have nothing to do :-)

>
> I think the patch is OK, though I would do some reordering, e.g. "sym->module"
> is not needed as the whole block is enclosed in such a check.
>
> I also would move the block down into
>        if (gsym && gsym->ns && gsym->type == GSYM_MODULE)
> which could be changed into
>        if (!gsym || (gsym && gsym->ns && gsym->type == GSYM_MODULE))
>
> And
> -        gfc_find_symbol (sym->name, gsym->ns, 0, &s);
> +         if (gsym)
> +          gfc_find_symbol (sym->name, gsym->ns, 0, &s);
>
> And then placing the "if (gsym == NULL)" in the
> +       else if (s == NULL)
> block.

All done.

> (In reply to comment #30)
>> The bus error reported in comment #21 has disappeared from my scope between
>> revisions 167584 and 167726;-(
>
> Ditto here, but looking at the dump, I see that the underlying issue is still
> present - and it is fixed by the patch.
>
>  * * *
>
> I will do some more tests.

Let me know when you have done them.  What about a testcase?  I guess
that since it was a testsuite failure we do not need one?
>
> Related to those whole-file DECL issues: I wonder why there are still
> differences between -flto and -fno-lto on Polyhedron (result is correct but
> runtimes are different) and for Jack (-O3 miscompilation of xplor-nih's slink.f
> and tenso.f, unless -fno-whole-file is used; -fno-inline-functions does not
> help).

Is there a PR for Jack's problem.

Thanks for looking at the patch

Paul

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[pault at gcc dot gnu.org]

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

--- Comment #33 from Paul Thomas <pault at gcc dot gnu.org> 2011-02-20 16:23:57 UTC ---
Author: pault
Date: Sun Feb 20 16:23:50 2011
New Revision: 170337

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170337
Log:
2011-02-20  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/45077
    PR fortran/44945
    * trans-types.c (gfc_get_derived_type): Remove code that looks
    for decls in gsym and add call to gfc_get_module_backend_decl.
    * trans.h : Add prototype for gfc_get_module_backend_decl.
    * trans-decl.c (gfc_get_module_backend_decl): New function.
    (gfc_get_symbol_decl): Call it.

2011-02-20  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/45077
    PR fortran/44945
    * gfortran.dg/whole_file_28.f90 : New test.
    * gfortran.dg/whole_file_29.f90 : New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/whole_file_28.f90
    trunk/gcc/testsuite/gfortran.dg/whole_file_29.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-types.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/44945] [4.6 Regression] Wrong decl for module vars / FAIL: gfortran.dg/char_array_structure_constructor.f90

[pault at gcc dot gnu.org]

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

Paul Thomas <pault at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #34 from Paul Thomas <pault at gcc dot gnu.org> 2011-02-20 16:28:09 UTC ---
I think that we can close this one now.

Thanks to all involved in reporting and fixing.

Fixed on trunk.

Paul