[PATCH] fixes Fortran parameter passing problem
Wolfgang Gellerich
gellerich@de.ibm.com
Tue Sep 12 12:39:00 GMT 2006
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,
More information about the Gcc-patches
mailing list