public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tobias Burnus <tobias@codesourcery.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>,
	fortran <fortran@gcc.gnu.org>,
	Paul Richard Thomas <paul.richard.thomas@gmail.com>,
	Thomas Koenig <tkoenig@netcologne.de>
Cc: Sandra Loosemore <sandra@codesourcery.com>
Subject: Re: [Patch] [v2] Fortran: Fix Bind(C) Array-Descriptor Conversion (Move to Front-End Code)
Date: Sat, 9 Oct 2021 23:55:07 +0200	[thread overview]
Message-ID: <8dab564d-ce83-0598-3549-e565898de8f6@codesourcery.com> (raw)
In-Reply-To: <44912f59-4679-7a35-a8bc-4c57b3a9e216@codesourcery.com>

[-- Attachment #1: Type: text/plain, Size: 5856 bytes --]

Re-sent with gzipped attachment as gcc-patches@ did reject it as being too large.
Alternatively, you can find it at
https://gcc.gnu.org/pipermail/fortran/attachments/20211009/dc26f92d/attachment-0001.bin
as fortran@ contrary to gcc-patches@ did accept the patch ...

Tobias

On 09.10.21 23:48, Tobias Burnus wrote:
> Hi all,
>
> attached is the updated version. Changes:
> * Handle noncontiguous arrays – with BIND(C), (g)Fortran needs to make it
>   contiguous in the caller but also handle noncontiguous in the callee.
> * Fixes/handle 'character(len=*)' with BIND(C); those always use an
>   array descriptor - also with explicit-size and assumed-size arrays
> * Fixed a bunch of bugs, found when writing extensive testcases.
> * Fixed type(*) handling - those now pass properly type and elem_len
>   on when calling a new function (bind(C) or not).
>
> Besides adding the type itself (which is rather straight forward),
> this patch only had minor modifications – and then the two big
> conversion functions.
>
> While it looks intimidating, it should be comparably simple to
> review as everything is on one place and hopefully sufficiently
> well documented.
>
> OK – for mainline?  Other comments? More PRs which are fixed?
> Issues not yet fixed (which are inside the scope of this patch)?
>
> (If this patch is too long, I also have a nine-day old pending patch
> at https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580624.html )
>
> Tobias
>
> PS: The following still applies.
>
> On 06.09.21 12:52, Tobias Burnus wrote:
>> gfortran's internal array descriptor (xgfc descriptor) and
>> the descriptor used with BIND(C) (CFI descriptor, ISO_Fortran_binding.h
>> of TS29113 / Fortran 2018) are different. Thus, when calling a BIND(C)
>> procedure the gfc descriptor has to be converted to cfi – and when a
>> BIND(C) procedure is implemented in Fortran, the argument has to be
>> converted back from CFI to gfc.
>>
>> The current implementation handles part in the FE and part in
>> libgfortran,
>> but there were several issues, e.g. PR101635 failed due to alias issues,
>> debugging wasn't working well, uninitialized memory was used in some
>> cases
>> etc.
>>
>> This patch now moves descriptor conversion handling to the FE – which
>> also
>> can make use of compile-time knowledge, useful both for diagnostic
>> and to
>> optimize the code.
>>
>> Additionally:
>> - Some cases where TS29113 mandates that the array descriptor should be
>>   used now use the array descriptor, in particular character scalars
>> with
>>   'len=*' and allocatable/pointer scalars.
>> - While debugging the alias issue, I simplified 'select rank'. While
>> some
>>   special case is needed for assumed-shape arrays, those cannot
>> appear when
>>   the argument has the pointer or allocatable attribute. That's not
>> only a
>>   missed optimization, pointer/allocatable arrays can also be NULL -
>> such
>>   that accessing desc->dim.ubound[rank-1] can be uninitialized memory
>> ...
>>
>> OK?  Comments? Suggestions?
>>
>>  * * *
>>
>> For some more dumps, see the discussion about the alias issue at:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578364.html
>> ("[RFH] ME optimizes variable assignment away / Fortran bind(C)
>> descriptor conversion")
>> plus the original emails:
>> - https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578271.html
>> - and (correct dump)
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578274.html
>>
>> Debugging - not ideal but not too bad either. For
>>   subroutine f(x) bind(C)
>>     integer :: x(:)
>> with an uninitialized size-4 array as argument:
>>
>> m::f (_x=...) at foo4.f90:3
>> 3       subroutine f(x) bind(C)
>> (gdb) p x
>> Cannot access memory at address 0x38
>> (gdb) p _x
>> $6 = ( base_addr = 0x7fffffffe2c0, elem_len = 4, version = 1, rank =
>> 1 '\001', attribute = 2 '\002', type = 1025, dim = (( lower_bound =
>> 0, extent = 5, sm = 4 )) )
>> (gdb) s
>> 5         x(1) = 5
>> (gdb) p x
>> $7 = (0, 0, 0, -670762413, 0)
>>
>>
>> Tobias
>>
>> PS: This patch fixes but not necessarily fully the following PRs:
>> PR fortran/102086 - [F2008][TS29113] Accepts invalid scalar TYPE(*)
>> as actual argument to assumed-rank
>> PR fortran/92189 - Fortran-written bind(C) function with allocatable
>> argument does not update C descriptor on exit
>> PR fortran/92621 - Problems with memory handling with allocatable
>> intent(out) arrays with bind(c)
>> PR fortran/101308 - Bind(C): gfortran does not create C descriptors
>> for scalar pointer/allocatable arguments
>> PR fortran/101635 - FAIL: gfortran.dg/PR93963.f90 – alias-handling
>> issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
>> PR fortran/92482 - BIND(C) with array-descriptor mishandled for type
>> character
>> and possibly some more.
>>
>> PPS: I should add some additional testcases – I try to do this as
>> Part 2 of this patch.
>>
>> PPPS: Once the patch is in, some audit needs to be done which parts
>> of those PRs remain
>> as follow-up work. I think some still existing issues are covered by
>> José's pending
>> patches + for those which are now fixed, the testcase might still be
>> added.
>>
>> -----------------
>> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße
>> 201, 80634 München; Gesellschaft mit beschränkter Haftung;
>> Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der
>> Gesellschaft: München; Registergericht München, HRB 106955
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: omp-descriptor-conv-v6.diff.gz --]
[-- Type: application/gzip, Size: 51996 bytes --]

  parent reply	other threads:[~2021-10-09 21:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06 10:52 [Patch] " Tobias Burnus
2021-09-10 18:48 ` PING – " Tobias Burnus
     [not found] ` <44912f59-4679-7a35-a8bc-4c57b3a9e216@codesourcery.com>
2021-10-09 21:55   ` Tobias Burnus [this message]
2021-10-13 16:01   ` [Patch] [v3] " Tobias Burnus
2021-10-13 20:10     ` Harald Anlauf
2021-10-17 15:59       ` Paul Richard Thomas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8dab564d-ce83-0598-3549-e565898de8f6@codesourcery.com \
    --to=tobias@codesourcery.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=paul.richard.thomas@gmail.com \
    --cc=sandra@codesourcery.com \
    --cc=tkoenig@netcologne.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).