Wolfgang Gellerich - [PATCH] fixes Fortran parameter passing problem (original) (raw)

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

• From: Wolfgang Gellerich
• To: fortran at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
• Date: Tue, 12 Sep 2006 14:39:41 +0200
• Subject: [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,