From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 931 invoked by alias); 8 Jan 2013 20:07:47 -0000 Received: (qmail 912 invoked by uid 22791); 8 Jan 2013 20:07:46 -0000 X-SWARE-Spam-Status: No, hits=-5.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-lb0-f175.google.com (HELO mail-lb0-f175.google.com) (209.85.217.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 Jan 2013 20:07:34 +0000 Received: by mail-lb0-f175.google.com with SMTP id gg13so674721lbb.20 for ; Tue, 08 Jan 2013 12:07:32 -0800 (PST) MIME-Version: 1.0 Received: by 10.152.134.243 with SMTP id pn19mr62477111lab.11.1357675652090; Tue, 08 Jan 2013 12:07:32 -0800 (PST) Received: by 10.152.133.14 with HTTP; Tue, 8 Jan 2013 12:07:31 -0800 (PST) In-Reply-To: <50E9ABC7.4040705@net-b.de> References: <50DCCC29.6010206@net-b.de> <50E01A7C.2090106@net-b.de> <50E9ABC7.4040705@net-b.de> Date: Tue, 08 Jan 2013 20:07:00 -0000 Message-ID: Subject: Re: [Patch, Fortran] PR55758 - Non-C_Bool handling with BIND(C) From: Janne Blomqvist To: Tobias Burnus Cc: gcc patches , gfortran Content-Type: text/plain; charset=UTF-8 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-01/txt/msg00439.txt.bz2 On Sun, Jan 6, 2013 at 6:52 PM, Tobias Burnus wrote: > ** ping ** > > Attached is a small variation, which in addition handles the case that a > non-BOOL_C LOGICAL, Bind(C) dummy argument (or result variable) is used in a > procedure call. In that case, the variable is now converted to a > TYPE_PRECISION == 1 variable. -- The updated patch was build and regtested > successfully. Nice, this should fix a pitfall with the previous patch. I still worry about these almost-but-not-quite logicals causing weird and very hard to track down bugs. A slightly safer variant of the approach you describe above would be to convert the variable directly after the bind(c) procedure call; that should make it pretty fool-proof, AFAICS? (in some cases that would be a bit of extra useless work, but I doubt it would matter performance-wise). > As written before, I believe that the patch avoids some pitfalls with C > interoperability of logical variables: On one hand, it improves > cross-compiler portability by rejecting non C_BOOL ones with > -std=f2003/f2008/f2008ts; This part is certainly ok. > on the other hand, it makes wrong-code issues due > to using non-0/1 integers from C much less likely. In both cases, the > type-precision==1 handling for non-BIND(C) Fortran LOGICALs or for Bind(C) > LOGICAL(kind=C_BOOL) remains the same; hence, no optimization issue is > caused. > > > OK for the trunk? > > Tobias > > PS: If there is consensus that this patch is a bad idea, I propose to reject > non-C_BOOL LOGICALs unconditionally as dummy argument or result variable of > BIND(C) procedures. Or do you have a better suggestion? > > > > On December 30, 2012, Tobias Burnus wrote: >> >> Janne Blomqvist wrote: >>> >>> On Fri, Dec 28, 2012 at 12:31 AM, Tobias Burnus wrote: >>>> >>>> a) The Fortran standard only defines LOGICAL(kind=C_Bool) as being >>>> interoperable with C - no other LOGICAL type. That matches GCC: With gcc >>>> (the C compiler) only _Bool is a BOOLEAN_TYPE with TYPE_PRECISION == 1. >>>> Hence, this patch rejects other logical kinds as dummy argument/result >>>> variable in BIND(C) procedures if -std=f2003/f2008/f2008ts is specified >>>> (using -pedantic, one gets a warning). >>> >>> Sorry, I don't understand, what is the -pedantic warning about if it's >>> already rejected? Or do you mean std=gnu -pedantic? >> >> >> The latter. Actually, I use "gfc_notify_std(GFC_STD_GNU, ..." and just >> observed the -pedantic result. (I have to admit that I never quite >> understood - and still don't - what -pedantic exactly does.) >> >>>> b) As GNU extension, other logical kinds are accepted in BIND(C) >>>> procedures; >>>> however, as the main use of "LOGICAL(kind=4)" (for BIND(C) procedures) >>>> is to >>>> handle logical expressions which use C's int, one has to deal with all >>>> integer values and not only 0 and 1. Hence, a normal integer type is >>>> used >>>> internally in that case. That has been done to avoid surprises of users >>>> and >>>> hard to trace bugs. >>> >>> Does this actually work robustly? >> >> >> I think it does in the sense that it mitigates the problems related to >> LOGICAL(kind=4) and BIND(C) procedures. No, if one thinks of it as full cure >> for the problem. The only way to ensure this is to turn all of gfortran's >> LOGICALs into integers - and even that won't prevent issues related to >> interoperability with C's _Bool as that one expects only 0 and 1. Thus, >> either C<->Fortran or Fortran <-> Fortran logical(kind=C_Bool) could still >> lead to problems. >> >>> E.g. if you have a logical but really integer under the covers, what >>> happens if you equivalence it with a "normal" logical variable. >> >> >> Well, equivalencing of actual arguments / result variables is not allowed >> (I think, not checked). Besides, if you have equivalenced two variables, if >> you have set one, you may not access the other, e.g.: >> >> logical :: A >> integer :: B >> equivalence (A,B) >> A = .true. >> B = 1 >> if (A) ... >> >> is invalid as "A" is not defined, even if A = .true. and B = 1 have >> exactly the same storage size and bit patterns and, hence, in practice "A" >> would be a well defined .true. >> >>> Or pass it as an argument to a procedure expecting a normal logical etc. >> >> >> If the value is only 1 or 0, there shouldn't be any problems. Only if one >> uses in turn ".not. dummy" there might be one. >> >> The idea of the patch was only to mitigate the problems - a full cure is >> not possible (cf. above). I think the most likely problematic code is >> if (.not. c_function()) >> which is fixed by the patch. And the hope is that fold-converting to a >> type-precision=1, Boolean-type logical fixes most of the remaining issues. >> >> I think the current solution which only affects non-C_BOOL-kind actual >> arguments and result variables of BIND(C) procedures is a good compromise. >> >> * * * >> >> But if others do not like this approach, one could turn the gfc_notify_std >> into a gfc_error are completely reject logicals with kinds /= C_Bool for >> dummy arguments/result variables in BIND(C) procedures. Would you prefer >> that approach? >> >> (Doing so will break user code (e.g. Open MPI) and make users unhappy but >> it will be a tad safer as the current patch.) >> >> Tobias >> > -- Janne Blomqvist