This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] fixes Fortran parameter passing problem


Hi F90 friends!

While debugging Fortran testcase char_result_6.f90 which crashed on
s390 I found a general problem in the Fortran front end. It concerns
the way how character string parameters are converted before passing
them. The problem occurs when optimizing with option -O2. Its seems to
hit s390 only but potentially affects other platforms as well.

Here is a simplified version of testcase char_result_6.f90:

----------------------------

program main
  implicit none

  character, target :: ch
  character, pointer :: chp

  ch = '1'
  chp => ch

  call test (f5 (chp), 11)

contains

  function f5 (c)
    character, pointer :: c
    character (len = scan ('123456789', c) + 10) :: f5
    f5 = ''
  end function f5

  subroutine test (string, length)
    character (len = *) :: string
    integer, intent (in) :: length
    if (len (string) .ne. length) call abort
  end subroutine test
end program main

----------------------------

which compiles to the following code:

----------------------------

(lines removed)
MAIN__:
.LFB2:
	stmg	%r6,%r15,48(%r15)
.LCFI0:
	lghi	%r2,70
	aghi	%r15,-176
.LCFI1:
	lghi	%r3,127
	lghi	%r4,0
	brasl	%r14,_gfortran_set_std
	la	%r12,175(%r15)
	larl	%r3,.LC0
	lghi	%r4,1
	lg	%r5,160(%r15)
	lghi	%r6,0
	lghi	%r2,9
	mvi	175(%r15),49
	stg	%r12,160(%r15)
	brasl	%r14,_gfortran_string_scan
        (lines removed)

----------------------------

Parameters are passed to _gfortran_string_scan using registers r2 to
r5. The bug is that register r5 is loaded from a stack location before
that location is written.

Tracing back what optimizations gcc performed revealed that the
scheduling pass interchanges insns 18 and 24, respectively. The
136r.sched1 pass delivers this output:

----------------------------

(lines deleted)
(insn 24 23 21 2 (set (reg:DI 5 %r5)
        (mem/f:DI (plus:DI (reg/f:DI 34 %fp)
                (const_int -16 [0xfffffffffffffff0])) [9 S8 A64])) 51 {*movdi_64} (nil)
    (expr_list:REG_EQUAL (plus:DI (reg/f:DI 34 %fp)
            (const_int -1 [0xffffffffffffffff]))
        (nil)))

(insn 21 24 25 2 (set (reg:DI 2 %r2)
        (const_int 9 [0x9])) 51 {*movdi_64} (nil)
    (nil))

(insn 25 21 15 2 (set (reg:DI 6 %r6)
        (const_int 0 [0x0])) 51 {*movdi_64} (nil)
    (nil))

(insn 15 25 18 2 (set (mem/s:QI (plus:DI (reg/f:DI 34 %fp)
                (const_int -1 [0xffffffffffffffff])) [2 ch+0 S1 A8])
        (const_int 49 [0x31])) 62 {*movqi} (nil)
    (nil))

(insn 18 15 26 2 (set (mem/c/i:DI (plus:DI (reg/f:DI 34 %fp)
                (const_int -16 [0xfffffffffffffff0])) [4 chp+0 S8 A64])
        (reg/f:DI 50 [ ch.1 ])) 51 {*movdi_64} (nil)
    (nil))

(call_insn 26 18 29 2 (parallel [
            (set (reg:DI 2 %r2)
                (call (mem:QI (symbol_ref:DI ("_gfortran_string_scan") [flags 0x41] <function_decl 0x200002bca00 _gfortran_string_scan>) [0 S1 A8])
                    (const_int 0 [0x0])))
            (clobber (reg:DI 14 %r14))
        ]) 413 {*brasl_r} (nil)
    (expr_list:REG_DEAD (reg:DI 3 %r3)
        (expr_list:REG_DEAD (reg:DI 4 %r4)
            (expr_list:REG_DEAD (reg:DI 5 %r5)
                (expr_list:REG_UNUSED (reg:DI 14 %r14)
                    (nil)))))
    (expr_list:REG_DEP_TRUE (use (reg:DI 6 %r6))
        (expr_list:REG_DEP_TRUE (use (reg:DI 5 %r5))
            (expr_list:REG_DEP_TRUE (use (reg:DI 4 %r4))
                (expr_list:REG_DEP_TRUE (use (reg:DI 3 %r3))
                    (expr_list:REG_DEP_TRUE (use (reg:DI 2 %r2))
                        (nil)))))))
(lines deleted)

----------------------------

while the order was ok after the previous pass 134r.life2:

----------------------------

(lines deleted)
(insn 18 17 21 2 (set (mem/c/i:DI (plus:DI (reg/f:DI 34 %fp)
                (const_int -16 [0xfffffffffffffff0])) [4 chp+0 S8 A64])
        (reg/f:DI 50 [ ch.1 ])) 51 {*movdi_64} (insn_list:REG_DEP_TRUE 17 (nil))
    (nil))

(insn 21 18 22 2 (set (reg:DI 2 %r2)
        (const_int 9 [0x9])) 51 {*movdi_64} (nil)
    (nil))

(insn 22 21 23 2 (set (reg:DI 3 %r3)
        (symbol_ref/f:DI ("*.LC0") [flags 0x2] <string_cst 0x200002e9780>)) 49 {*movdi_larl} (nil)
    (nil))

(insn 23 22 24 2 (set (reg:DI 4 %r4)
        (const_int 1 [0x1])) 51 {*movdi_64} (nil)
    (nil))

(insn 24 23 25 2 (set (reg:DI 5 %r5)
        (mem/f:DI (plus:DI (reg/f:DI 34 %fp)
                (const_int -16 [0xfffffffffffffff0])) [9 S8 A64])) 51 {*movdi_64} (nil)
    (expr_list:REG_EQUAL (plus:DI (reg/f:DI 34 %fp)
            (const_int -1 [0xffffffffffffffff]))
        (nil)))

(insn 25 24 26 2 (set (reg:DI 6 %r6)
        (const_int 0 [0x0])) 51 {*movdi_64} (nil)
    (nil))

(call_insn 26 25 27 2 (parallel [
            (set (reg:DI 2 %r2)
                (call (mem:QI (symbol_ref:DI ("_gfortran_string_scan") [flags 0x41] <function_decl 0x200002bca00 _gfortran_string_scan>) [0 S1 A8])
                    (const_int 0 [0x0])))
            (clobber (reg:DI 14 %r14))
        ]) 413 {*brasl_r} (insn_list:REG_DEP_TRUE 21 (insn_list:REG_DEP_TRUE 22 (insn_list:REG_DEP_TRUE 23 (insn_list:REG_DEP_TRUE 24 (insn_list:REG_DEP_TRUE 25 (nil))))))
    (expr_list:REG_UNUSED (reg:DI 14 %r14)
        (expr_list:REG_DEAD (reg:DI 3 %r3)
            (expr_list:REG_DEAD (reg:DI 4 %r4)
                (expr_list:REG_DEAD (reg:DI 5 %r5)
                    (expr_list:REG_UNUSED (reg:DI 14 %r14)
                        (nil))))))
    (expr_list:REG_DEP_TRUE (use (reg:DI 6 %r6))
        (expr_list:REG_DEP_TRUE (use (reg:DI 5 %r5))
            (expr_list:REG_DEP_TRUE (use (reg:DI 4 %r4))
                (expr_list:REG_DEP_TRUE (use (reg:DI 3 %r3))
                    (expr_list:REG_DEP_TRUE (use (reg:DI 2 %r2))
                        (nil)))))))
(lines deleted)

----------------------------

The modification perfromed by the scheduler seems to be allowed since
the alias sets of the memory locations accessed differ (4 and 9,
respectively).  The reason for the different alias set numbers is that
the same stack variable is pointed to by pointers with different
target types which is obvious from 102t.final_cleanup:

----------------------------

(lines deleted)
  ch[1]{lb: 1 sz: 1} = 49;
  ch.1 = (char[1:1] *) &ch;
  chp = ch.1;
  D.901 = (char[1:] * *) &chp;
  D.903 = _gfortran_string_scan (9, "123456789", 1, *D.901, 0);
(lines deleted)

----------------------------

Here, the type of the actual parameter is pointer to char[1:1] which
is then casted to a pointer to char[1:].  The bug fix is not to do the
type conversion via a pointer type cast but rather call fold_convert
which, in this case, performs the conversion via a NOP_EXPR.


Validation:

The regression test on s390 shows that char_result6.f90 now compiles
and executes correctly and there are no other changes.  The patch
caused no changes on an Intel system.

Regards, Wolfgang




*** trans-expr.c-ORI	Tue Sep 12 13:23:11 2006
--- trans-expr.c	Tue Sep 12 13:25:10 2006
*************** gfc_add_interface_mapping (gfc_interface
*** 1386,1396 ****
        tmp = gfc_get_character_type_len (sym->ts.kind, NULL);
        tmp = build_pointer_type (tmp);
        if (sym->attr.pointer)
! 	tmp = build_pointer_type (tmp);
! 
!       value = fold_convert (tmp, se->expr);
!       if (sym->attr.pointer)
! 	value = build_fold_indirect_ref (value);
      }
  
    /* If the argument is a scalar, a pointer to an array or an allocatable,
--- 1386,1395 ----
        tmp = gfc_get_character_type_len (sym->ts.kind, NULL);
        tmp = build_pointer_type (tmp);
        if (sym->attr.pointer)
!         value = build_fold_indirect_ref (se->expr);
!       else
!         value = se->expr;
!       value = fold_convert (tmp, value);
      }
  
    /* If the argument is a scalar, a pointer to an array or an allocatable,


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]