On Sun, Feb 17, 2019 at 7:19 PM Thomas Koenig <[email protected]> wrote:
>
> Hello world,
>
> the attached patch fixes a rather bad ABI violation on POWER systems.
>
> In the absence of an explicit interface and if a procedure is not in
> the same file, gfortran currently generates wrong function decls -
> a longstanding problem that also creates problems with LTO, because
> it (correctly) complains about mismatched declarations.
>
> Usually, we got lucky because the actual calling sequences generated
> by the compiler with the wrong info happened to match the ones
> with the correct info. However, our luck ran out on POWER because
> as soon as arguments were passed in memory, things did not work
> any more. The test case in question (see attachments) produced
> wrong code on POWER, but merely warned with LTO on other systems.
>
> The method for solving this problem can be seen in the patch - if
> there is no backend decl for an external procedure, simply generate
> a formal argument list from the arguments.
>
> Regression tests turned up a few ICEs (now fixed), plus two
> very invalid test cases, which I think are not worth saving.
They were added to verify we don't ICE for such invalid testcases.
How do they fail now?
> I suspect that this will also fix a few LTO bugs, but we can always
> check that after this has been committed.
>
> I'd still like confirmation from one of the POWER people that
> this also fixes the bug on that architecture.
>
> Should this still go into gcc-9? Richard has indicated in the
> PR that he thinks so. I think so too, because of the severity
> of the bug(s) this fixes. Any bugs resulting from this could
> be either a) ICE-on-valid (easily fixed) or b) somehow generating
> a wrong decl, but we are already doing this as of this moment,
> so things can not really be made much worse, and a lot better.
>
> So, ok for trunk and for backport (with some time in between)
> once gcc-8 has re-opened?
The patch looks good to me. I wonder how the frontend handles
the 2nd call to doesntwork_p8 for
program main
implicit none
character :: c
character(len=20) :: res, doesntwork_p8
external doesntwork_p8
c = 'o'
res = doesntwork_p8(c,1,2,3,4,5,6)
res = doesntwork_p8(1,2)
if (res /= 'foo') stop 3
end program main
at least I get no diagnostic and the patch suggests whatever call
gets there first determines the backend-decl and its type? At least
when I omit res = from the second call I get
t.f:4:47:
4 | character(len=20) :: res, doesntwork_p8
| 1
......
8 | call doesntwork_p8(1,2)
|
Error: ‘doesntwork_p8’ at (1) has a type, which is not consistent with
the CALL at (2)
Does the Fortran standard say anything in how the above case should be handled?
Richard.
>
> Regards
>
> Thomas
>
> 2019-02-17 Thomas Koenig <[email protected]>
>
> PR fortran/87689
> * trans-decl.c (gfc_get_extern_function_decl): Add argument
> actual_args and pass it through to gfc_get_function_type.
> * trans-expr.c (conv_function_val): Add argument actual_args
> and pass it on to gfc_get_extern_function_decl.
> (conv_procedure_call): Pass actual arguments to conv_function_val.
> * trans-types.c (get_formal_from_actual_arglist): New function.
> (gfc_get_function_type): Add argument actual_args. Generate
> formal args from actual args if necessary.
> * trans-types.h (gfc_get_function_type): Add optional argument.
> * trans.h (gfc_get_extern_function_decl): Add optional argument.
>
> 2019-02-17 Thomas Koenig <[email protected]>
>
> PR fortran/87689
> * gfortran.dg/lto/20091028-1_0.f90: Remove invalid test
> case.
> * gfortran.dg/lto/20091028-1_1.c: Likewise.
> * gfortran.dg/lto/20091028-2_0.f90: Likewise.
> * gfortran.dg/lto/20091028-2_1.c: Likewise.
> * gfortran.dg/lto/pr87689_0.f: New file.
> * gfortran.dg/lto/pr87689_1.f: New file.
