On 5/17/19 10:48 AM, Jeff Law wrote:
On 5/16/19 2:09 AM, Jakub Jelinek wrote:
Hi!

Fortran subroutines/functions that have CHARACTER arguments have also
hidden arguments at the end of the argument list which hold the string
length.  This is something all Fortran compilers I've tried do and is
done in order to support calling unprototyped subroutines/functions
where one can't know if the callee is using character(len=constant)
or character(len=*) argument, where for the latter one has to pass
the extra argument because there is no other way to propagate that info,
while for the former it is kind of redundant but still part of the ABI;
the compiler just uses the constant directly instead of asking about the
real passed string length.

Another thing is that until PR87689 has been fixed, the Fortran FE has been
broken, used vararg-ish prototypes in most cases and that prevented (all?)
tail call optimizations in the Fortran code.

Apparently it is a common case that buggy C/C++ wrappers around the Fortran
functions just ignore the ABI and don't pass the hidden string length
arguments at all, which seemed to work fine in gfortran until recently
(because the arguments weren't used).  When we started making tail calls
from such functions, this has of course changed, because when making a tail
call we overwrite the caller's argument slots with the arguments we want to
pass to the callee.  Not really sure why it seemed to work with other
compilers; trying https://fortran.godbolt.org/z/ckMt1t
subroutine foo (a, b, c, d, e, f, g, h)
   integer :: b, c, d, e, f, g, h
   character(len=1) :: a
   call bar (a, b, c, d, e, f, g, h)
end subroutine foo
both older/new gfortran and ifort 19.0.0 emit a tail call which overwrites
the hidden string length argument with 1, but when trying the
https://fortran.godbolt.org/z/xpsH8e LAPACK routine, ifort for some reason
doesn't tail call it.

I'm not really happy to provide workarounds for undefined behavior,
especially because that will mean it might take longer if ever if those
buggy programs are fixed.  On the other side, the PR87689 bug fix has been
backported to all release branches and so now not only trunk, but also 9.1,
8.3.1 and 7.4.1 are affected.  Instead of trying to disable all tail calls,
this patch disables tail calls from functions/subroutines that have those
hidden string length arguments and don't use character(len=*) (in that case
the function wouldn't seem to work previously either, because the argument
is really used), where those hidden string length arguments are passed
on the stack and where the tail callee also would want to pass arguments
on the stack (if we spent even more time on this, we could narrow it down
further and check if the tail call would actually store anything overlapping
the hidden string length arguments on the stack).

This workaround probably needs guarding with some Fortran FE specific
option, so that it can be disabled, will defer that to the Fortran
maintainers.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
release branches (not sure about LTO on the release branches, does one need
to bump anything when changing the LTO format by streaming another bit)?

2019-05-16  Jakub Jelinek  <ja...@redhat.com>

        PR fortran/90329
        * tree-core.h (struct tree_decl_common): Document
        decl_nonshareable_flag for PARM_DECLs.
        * tree.h (DECL_HIDDEN_STRING_LENGTH): Define.
        * calls.c (expand_call): Don't try tail call if caller
        has any DECL_HIDDEN_STRING_LENGTH PARM_DECLs that are or might be
        passed on the stack and callee needs to pass any arguments on the
        stack.
        * tree-streamer-in.c (unpack_ts_decl_common_value_fields): Use
        else if instead of series of mutually exclusive ifs.  Handle
        DECL_HIDDEN_STRING_LENGTH for PARM_DECLs.
        * tree-streamer-out.c (pack_ts_decl_common_value_fields): Likewise.

        * trans-decl.c (create_function_arglist): Set
        DECL_HIDDEN_STRING_LENGTH on hidden string length PARM_DECLs if
        len is constant.
My first, second and third thought has been we shouldn't be catering to
code that is so clearly broken.  Maybe we could do this on the release
branches which would in turn give folks roughly a year to fix up this mess?

jeff


Not that I have much say, but I have been following this thread and the others about broken wrappers and screwed up prototypes being used by R for there use of LAPACK. I have been holding off saying anything.

I don't thing we should be doing anything in gfortran at all. I think the R people need to fix their calls. People get caught by not following the proper conventions and then want the compiler to fix it. Its not the compiler writers job to fix users bad code. The Fortran conventions of having the string lengths appended to the end has been known for many many years and well documented everywhere.

Sorry for ranting and I understand everyone is just trying to do the right thing. It would have been about an editorial fix on the R side.

Maybe Jeff as a good compromise here in that at least we would not have to carry it forward in time.

Jerry

Reply via email to