[Bug libfortran/24174] New: real(10) array output broken

[Bug libfortran/24174] New: real(10) array output broken

[fxcoudert at gcc dot gnu dot org]

The following program, which worked before Janne Blomqvist's recent patch to
enable array I/O, now fails:

$ cat u.f90 
  real(kind=10) :: a,b(1)
  a = 0
  b(:) = 0
  print *, a
  print *, b
  end
$ gfortran u.f90 && ./a.out
   0.00000000000000      
At line 5 of file u.f90
Internal Error: bad real kind


It appears that, at line 1005 in transfer.c, the transfer routine is called
with kind=12 while it was (and probably should be) 10. Somewhere, the
distinction between kind and size has to be made.


-- 
           Summary: real(10) array output broken
           Product: gcc
           Version: 4.1.0
            Status: NEW
          Keywords: rejects-valid
          Severity: normal
          Priority: P2
         Component: libfortran
        AssignedTo: jblomqvi at cc dot hut dot fi
        ReportedBy: fxcoudert at gcc dot gnu dot org


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #1 from jblomqvi at cc dot hut dot fi  2005-10-03 09:39 -------
Ah, yes. Also see PR22519 and PR23419. Which way do we want it for unformatted?
Should kind=10 use 10 bytes of storage or should it use whatever the padded
size is? I would prefer the padded size, as otherwise we take a x10 speed hit
when doing array IO.

But anyway, I'm pretty sure this bug is because the size in the array
descriptor is 12, since real(10) is padded to 12 bytes, right? Oh, how much
simpler life would be without padding.

I think it would be quite simple to kludge over this bug in the library, but if
we want a real solution, as in splitting the kind=size relationship I think
more work is needed.

I'll take a look tonight.


-- 

jblomqvi at cc dot hut dot fi changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |22519


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

[Bug libfortran/24174] real(10) array output broken

[fxcoudert at gcc dot gnu dot org]

------- Comment #2 from fxcoudert at gcc dot gnu dot org  2005-10-03 10:37 -------
(In reply to comment #1)
> Should kind=10 use 10 bytes of storage or should it use whatever the padded
> size is? I would prefer the padded size, as otherwise we take a x10 speed hit
> when doing array IO.

I do agree with you. 12 bytes will make our life much simpler.


-- 


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #3 from jblomqvi at cc dot hut dot fi  2005-10-03 19:37 -------
Created an attachment (id=9865)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=9865&action=view)
Patch for PR24174, also fixes formatted output for complex(10)

libgfortran Changelog:

2005-10-03  Janne Blomqvist <jblomqvi@cc.hut.fi>

        PR fortran/24174
        * io/io.h: Add argument to prototypes.
        * io/list_read.c (list_formatted_read): Add kind argument, cleanup. 
        * io/transfer.c: Add argument to transfer function pointer.
        (unformatted_read): Add kind argument.
        (unformatted_write): Likewise.
        (formatted_transfer_scalar): Fix for padding with complex(10).
        (formatted_transfer): Add kind argument, cleanup.
        (transfer_integer): Add kind argument to transfer call.
        (transfer_real): Likewise.
        (transfer_logical): Likewise.
        (transfer_character): Likewise.
        (transfer_complex): Likewise.
        (transfer_array): Likewise.
        (data_transfer_init): Add kind argument to formatted_transfer
        call.
        (iolength_transfer): Add kind argument, cleanup.
        * io/write.c (write_complex): Fix for padding with complex(10).
        (list_formatted_write): Add kind argument, cleanup.



-- 


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #4 from jblomqvi at cc dot hut dot fi  2005-10-03 19:40 -------
Created an attachment (id=9866)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=9866&action=view)
Testcase

And here is a test case to go with the previous patch.

Sorry my mail is b0rked at the moment so I can't send the patch via the mailing
list.

It should be noted that the patch assumes that the padding for real(10) is 10
bytes data + 2 bytes padding. This works on i686-Linux, might not work on other
targets (big endian?).


-- 


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

[Bug libfortran/24174] real(10) array output broken

[tkoenig at gcc dot gnu dot org]

------- Comment #5 from tkoenig at gcc dot gnu dot org  2005-10-07 20:12 -------

> It should be noted that the patch assumes that the padding for real(10) is 10
> bytes data + 2 bytes padding. This works on i686-Linux, might not work on other
> targets (big endian?).

Itanium has padding to 16 bytes for 10-byte reals.

Using 12 bytes for 10-byte reals on ia686 would mean breaking binary
compatibility with Itanium :-(


-- 

tkoenig at gcc dot gnu dot org changed:

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


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #6 from jblomqvi at cc dot hut dot fi  2005-10-09 19:35 -------
(In reply to comment #5)
> > It should be noted that the patch assumes that the padding for real(10) is 10
> > bytes data + 2 bytes padding. This works on i686-Linux, might not work on other
> > targets (big endian?).
> 
> Itanium has padding to 16 bytes for 10-byte reals.
> 
> Using 12 bytes for 10-byte reals on ia686 would mean breaking binary
> compatibility with Itanium :-(

Oh dear.. Is there any functionality one can use to get information about
padding (size, location of pad bytes etc.), or do we need to resort to lots of
#ifdef kludging?


-- 


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #7 from jblomqvi at cc dot hut dot fi  2005-10-09 20:52 -------
(In reply to comment #5)
> > It should be noted that the patch assumes that the padding for real(10) is 10
> > bytes data + 2 bytes padding. This works on i686-Linux, might not work on other
> > targets (big endian?).
> 
> Itanium has padding to 16 bytes for 10-byte reals.
> 
> Using 12 bytes for 10-byte reals on ia686 would mean breaking binary
> compatibility with Itanium :-(

Indeed, looking at
http://gcc.gnu.org/onlinedocs/gcc/i386-and-x86_002d64-Options.html#i386-and-x86_002d64-Options
it seems that x86-64 also pads real(10) to 16 bytes. See the doc for the
-m128bit-long-double switch.


-- 

jblomqvi at cc dot hut dot fi changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #8 from jblomqvi at cc dot hut dot fi  2005-10-09 21:29 -------
Well, a slightly less broken approach than the patch in #3 would be to use
sizeof(GFC_REAL_10) and sizeof(GFC_COMPLEX_10) instead of hardcoded sizes. But
the problem with this would still be 1) location of pad bytes and 2) changing
the ABI with -m128bit-long-double or similar switches.


-- 


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #9 from jblomqvi at cc dot hut dot fi  2005-10-11 06:11 -------
Updated patch here: http://gcc.gnu.org/ml/gcc-patches/2005-10/msg00572.html


-- 

jblomqvi at cc dot hut dot fi changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |ASSIGNED


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

[Bug libfortran/24174] real(10) array output broken

[jblomqvi at cc dot hut dot fi]

------- Comment #10 from jblomqvi at cc dot hut dot fi  2005-11-05 18:07 -------
Updated**2 patch here: http://gcc.gnu.org/ml/gcc-patches/2005-11/msg00348.html


-- 


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

[Bug libfortran/24174] real(10) array output broken

[jb at gcc dot gnu dot org]

------- Comment #11 from jb at gcc dot gnu dot org  2005-11-06 18:28 -------
Subject: Bug 24174

Author: jb
Date: Sun Nov  6 18:28:22 2005
New Revision: 106563

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=106563
Log:
gfortran ChangeLog

2005-11-06  Janne Blomqvist <jb@gcc.gnu.org>

        PR fortran/24174
        PR fortran/24305
        * fortran/trans-io.c (gfc_build_io_library_fndecls): Add kind
        argument to transfer_array.
        (transfer_array_desc): Add kind argument.

testsuite ChangeLog:

2005-11-06  Janne Blomqvist <jb@gcc.gnu.org>

        PR fortran/24174
        PR fortran/24305
        * testsuite/gfortran.dg/large_real_kind_form_io_1.f90: New file.

libgfortran Changelog:

2005-11-06  Janne Blomqvist <jb@gcc.gnu.org>

        PR fortran/24174
        PR fortran/24305
        * io/io.h: Add argument to prototypes, add prototypes for
        size_from_*_kind functions.
        * io/list_read.c (read_complex): Add size argument, use
        it. 
        (list_formatted_read): Add size argument, cleanup.
        (list_formatted_read_scalar): Add size argument.
        (nml_read_obj): Fix for padding.
        * io/transfer.c: Add argument to transfer function pointer.
        (unformatted_read): Add size argument.
        (unformatted_write): Likewise.
        (formatted_transfer_scalar): Fix for padding with complex(10).
        (formatted_transfer): Add size argument, cleanup.
        (transfer_integer): Add size argument to transfer call.
        (transfer_real): Likewise.
        (transfer_logical): Likewise.
        (transfer_character): Likewise.
        (transfer_complex): Likewise.
        (transfer_array): New kind argument, use it.
        (data_transfer_init): Add size argument to formatted_transfer
        call.
        (iolength_transfer): Add size argument, cleanup.
        * io/write.c (write_complex): Add size argument, fix for padding
        with complex(10).
        (list_formatted_write): Add size argument, cleanup.
        (list_formatted_write_scalar): Add size argument, use it.
        (nml_write_obj): Fix for size vs. kind issue.
        * io/size_from_kind.c: New file.
        * Makefile.am: Add io/size_from_kind.c.
        * configure: Regenerate.
        * Makefile.in: Regenerate.


Added:
    trunk/gcc/testsuite/gfortran.dg/large_real_kind_form_io_1.f90
    trunk/libgfortran/io/size_from_kind.c
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-io.c
    trunk/gcc/testsuite/ChangeLog
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/Makefile.am
    trunk/libgfortran/Makefile.in
    trunk/libgfortran/configure
    trunk/libgfortran/io/io.h
    trunk/libgfortran/io/list_read.c
    trunk/libgfortran/io/transfer.c
    trunk/libgfortran/io/write.c


-- 


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

[Bug libfortran/24174] real(10) array output broken

[jb at gcc dot gnu dot org]

------- Comment #12 from jb at gcc dot gnu dot org  2005-11-11 20:46 -------
This is fixed by the 2005-11-06 patch.


-- 

jb at gcc dot gnu dot org changed:

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


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

[Bug libfortran/24174] real(10) array output broken

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.1.0


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