46638 – Wrong result with TRANSFER from string to DT

Bug 46638 - Wrong result with TRANSFER from string to DT

Summary: Wrong result with TRANSFER from string to DT

Status:	RESOLVED FIXED

Alias:	None

Product:	gcc
Classification:	Unclassified
Component:	fortran (show other bugs)
Version:	4.6.0

Importance:	P3 normal
Target Milestone:	---
Assignee:	Not yet assigned to anyone

URL:
Keywords:	wrong-code

Depends on:
Blocks:	32834
	Show dependency tree / graph

Reported:	2010-11-24 08:15 UTC by Tobias Burnus
Modified:	2010-11-27 21:27 UTC (History)
CC List:	1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Tobias Burnus 2010-11-24 08:15:08 UTC

The following program of James Van Buskirk prints with gfortran 4.3 to 4.6:
0000000A 0000000A 0000000A 0000000A

Expected result (as with NAG, g95, ifort):
0000000A 756E6547 49656E69 6C65746E


Found at http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/1b907e3b7b6f3461

program test5
   use ISO_C_BINDING
   implicit none
   type, bind(C) :: CPUID_type
      integer(C_INT32_T) eax
      integer(C_INT32_T) ebx
      integer(C_INT32_T) edx
      integer(C_INT32_T) ecx
   end type CPUID_type
   type(CPUID_TYPE) result
result = transfer(achar(10)//achar(0)//achar(0)//achar(0)//'GenuineIntel',result)
write(*,'(4(z8.8:1x))') result
end program test5

Comment 1 Tobias Burnus 2010-11-24 08:41:15 UTC

More details: The run-time folding works; the issue is the compile time.

The call to
  gfc_target_encode_expr (source, buffer, buffer_size);
looks OK (buffer size = 16, source is the string.)

Up to gfc_encode_character is seems to be OK. I think something goes wrong in gfc_target_interpret_expr possibly in gfc_interpret_derived.

The dump looks as follows:
    struct cpuid_type cpuid_type.0;

    cpuid_type.0.eax = 10;
    cpuid_type.0.ebx = 10;
    cpuid_type.0.edx = 10;
    cpuid_type.0.ecx = 10;
    result = cpuid_type.0;

The problem seems to be that
    ptr = TREE_INT_CST_LOW (DECL_FIELD_OFFSET (cmp->backend_decl));

return "0" for all components:

(gdb) p debug_tree(result->ts.u.derived->components->backend_decl->field_decl->offset)
 <integer_cst 0x2aaaaab35410 type <integer_type 0x2aaaaab45000> constant 0>
$18 = void

(gdb) p debug_tree(result->ts.u.derived->components->next->backend_decl->field_decl->offset)
 <integer_cst 0x2aaaaab35410 type <integer_type 0x2aaaaab45000> constant 0>
$19 = void

Comment 2 Tobias Burnus 2010-11-24 08:50:24 UTC

I wonder whether it should be "bit_offset" (DECL_FIELD_BIT_OFFSET) instead of "offset" (DECL_FIELD_OFFSET).

Reading the comment in tree.h (see below), one probably should add the two:
   DECL_FIELD_OFFSET(...) + DECL_FIELD_BIT_OFFSET (...)/8


(gdb) p debug_tree(result->ts.u.derived->components->backend_decl->field_decl->bit_offset)
 <integer_cst 0x2aaaaab35b18 type <integer_type 0x2aaaaab450a8 bit_size_type> constant 0>
$22 = void

(gdb) p debug_tree(result->ts.u.derived->components->next->backend_decl->field_decl->bit_offset)
 <integer_cst 0x2aaaaab356e0 type <integer_type 0x2aaaaab450a8 bit_size_type> constant 32>
$23 = void

(gdb) p debug_tree(result->ts.u.derived->components->next->next->backend_decl->field_decl->bit_offset)
 <integer_cst 0x2aaaaab357a8 type <integer_type 0x2aaaaab450a8 bit_size_type> constant 64>


/* In a FIELD_DECL, this is the field position, counting in bytes, of the
   DECL_OFFSET_ALIGN-bit-sized word containing the bit closest to the beginning
   of the structure.  */
#define DECL_FIELD_OFFSET(NODE) (FIELD_DECL_CHECK (NODE)->field_decl.offset)


/* In a FIELD_DECL, this is the offset, in bits, of the first bit of the
   field from DECL_FIELD_OFFSET.  This field may be nonzero even for fields
   that are not bit fields (since DECL_OFFSET_ALIGN may be larger than the
   natural alignment of the field's type).  */
#define DECL_FIELD_BIT_OFFSET(NODE) \
  (FIELD_DECL_CHECK (NODE)->field_decl.bit_offset)

Comment 3 Tobias Burnus 2010-11-24 09:56:26 UTC

Patch:

diff --git a/gcc/fortran/target-memory.c b/gcc/fortran/target-memory.c
index 93e1c8c..1bca1dd 100644
--- a/gcc/fortran/target-memory.c
+++ b/gcc/fortran/target-memory.c
@@ -477,7 +477,8 @@ gfc_interpret_derived (unsigned char *buffer, size_t buffer_size, gfc_expr *resu
       /* The constructor points to the component.  */
       c->n.component = cmp;

-      ptr = TREE_INT_CST_LOW (DECL_FIELD_OFFSET (cmp->backend_decl));
+      ptr = TREE_INT_CST_LOW (DECL_FIELD_OFFSET (cmp->backend_decl))
+           + TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (cmp->backend_decl))/8;
       gfc_target_interpret_expr (&buffer[ptr], buffer_size - ptr, e);
     }

Comment 4 Tobias Burnus 2010-11-24 16:42:19 UTC

Author: burnus
Date: Wed Nov 24 16:42:06 2010
New Revision: 167119

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

        PR fortran/46638
        * target-memory.c (gfc_interpret_derived): Correctly handle
        component offset.

2010-11-24  Tobias Burnus  <burnus@net-b.de>

        PR fortran/46638
        * gfortran.dg/transfer_simplify_10.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/transfer_simplify_10.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/target-memory.c
    trunk/gcc/testsuite/ChangeLog

Comment 5 Tobias Burnus 2010-11-25 08:05:01 UTC

Author: burnus
Date: Thu Nov 25 08:04:46 2010
New Revision: 167138

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

        PR fortran/46638
        * target-memory.c (gfc_interpret_derived): Correctly handle
        component offset.

2010-11-25  Tobias Burnus  <burnus@net-b.de>

        PR fortran/46638
        * gfortran.dg/transfer_simplify_10.f90: New.


Added:
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/transfer_simplify_10.f90
Modified:
    branches/gcc-4_4-branch/gcc/fortran/ChangeLog
    branches/gcc-4_4-branch/gcc/fortran/target-memory.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog

Comment 6 Tobias Burnus 2010-11-25 09:02:54 UTC

Author: burnus
Date: Thu Nov 25 09:02:33 2010
New Revision: 167139

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

        PR fortran/46638
        * target-memory.c (gfc_interpret_derived): Correctly handle
        component offset.

2010-11-25  Tobias Burnus  <burnus@net-b.de>

        PR fortran/46638
        * gfortran.dg/transfer_simplify_10.f90: New.


Added:
    branches/gcc-4_5-branch/gcc/testsuite/gfortran.dg/transfer_simplify_10.f90
Modified:
    branches/gcc-4_5-branch/gcc/fortran/ChangeLog
    branches/gcc-4_5-branch/gcc/fortran/target-memory.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog

Comment 7 Tobias Burnus 2010-11-25 09:03:51 UTC

FIXED on the trunk (4.6) and the 4.4 and 4.5 branches.

Comment 8 Mikael Pettersson 2010-11-27 21:07:23 UTC

The test case added to 4.5 branch for this PR fails on powerpc64-linux and sparc64-linux, see <http://gcc.gnu.org/ml/gcc-testresults/2010-11/msg02300.html> which shows:

FAIL: gfortran.dg/transfer_simplify_10.f90  -O0  execution test
FAIL: gfortran.dg/transfer_simplify_10.f90  -O1  execution test
FAIL: gfortran.dg/transfer_simplify_10.f90  -O2  execution test
FAIL: gfortran.dg/transfer_simplify_10.f90  -O3 -fomit-frame-pointer  execution test
FAIL: gfortran.dg/transfer_simplify_10.f90  -O3 -fomit-frame-pointer -funroll-loops  execution test
FAIL: gfortran.dg/transfer_simplify_10.f90  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions  execution test
FAIL: gfortran.dg/transfer_simplify_10.f90  -O3 -g  execution test
FAIL: gfortran.dg/transfer_simplify_10.f90  -Os  execution test

According to gcc/testsuite/gfortran/gfortran.log the test case always outputs:

0A000000 47656E75 696E6549 6E74656C 61626364

Looking at the test case it seems to assume little-endian byte order, which isn't true here.

Also, the gcc/testsuite/ChangeLog update on 4.5 branch in r167139 is wrong, it's a copy of the gcc/fortran/ChangeLog entry while it should have listed the added test case (transfer_simplify_10.f90).

Comment 9 Tobias Burnus 2010-11-27 21:22:04 UTC

Author: burnus
Date: Sat Nov 27 21:22:00 2010
New Revision: 167209

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

        PR fortran/46638
        PR fortran/46668
        * gfortran.dg/transfer_simplify_10.f90: Fix endian issue.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/transfer_simplify_10.f90

Comment 10 Tobias Burnus 2010-11-27 21:26:19 UTC

Author: burnus
Date: Sat Nov 27 21:26:15 2010
New Revision: 167210

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

        PR fortran/46638
        PR fortran/46668
        * gfortran.dg/transfer_simplify_10.f90: Fix endian issue.


Modified:
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/gfortran.dg/transfer_simplify_10.f90

Comment 11 Tobias Burnus 2010-11-27 21:27:52 UTC

Author: burnus
Date: Sat Nov 27 21:27:49 2010
New Revision: 167211

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

        PR fortran/46638
        PR fortran/46668
        * gfortran.dg/transfer_simplify_10.f90: Fix endian issue.


Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/transfer_simplify_10.f90