public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug fortran/101635] New: FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
@ 2021-07-27  8:34 burnus at gcc dot gnu.org
  2021-07-27 13:29 ` [Bug fortran/101635] " pault at gcc dot gnu.org
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: burnus at gcc dot gnu.org @ 2021-07-27  8:34 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101635

            Bug ID: 101635
           Summary: FAIL: gfortran.dg/PR93963.f90 – alias-handling issue
                    with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                CC: jrfsousa at gcc dot gnu.org, sandra at gcc dot gnu.org
  Target Milestone: ---

This is kind of a regression as the testcase is new with the
commit r12-2511-g0cbf03689e3e7d9d6002b8e5d159ef3716d0404c
for PR 93308, PR 93963, PR 94327, PR 94331, and PR 97046.

The testcase  gfortran.dg/PR93963.f90  has the following;
the → lines are from the -fdump-tree-original:

  integer(kind=c_int),     pointer :: intp(:)
  nullify(intp)
  irnk = rank_p(intp)   ! this is BIND(C) with an assumed-rank argument

and the dump

   intp.dtype = {.elem_len=4, .rank=1, .type=1};
   intp.data = 0B;

and for the rank_p call:
      void * cfi.6;

      if ((integer(kind=4)[0:] *) intp.data == 0B)
        {
          intp.dtype = {.elem_len=4, .rank=1, .type=1};
        }
      _gfortran_gfc_desc_to_cfi_desc (&cfi.6, &intp);
      irnk = rank_p (cfi.6);
      __builtin_free (cfi.6);


With -m32  + optimizations, there seems to be an alias analysis issue which
causes that in rank_p, the argument's rank is <0 or >15 – such that rank_p
returns -1000. Unsurprisingly,
  -1000 != 1
and, hence, the program runs into:
  if (irnk /= rnk)        stop 1

 * * *

Solution:  I think the proper fix is to remove both 
_gfortran_gfc_desc_to_cfi_desc  and  _gfortran_cfi_desc_to_gfc_desc from
libgfortran/runtime/ISO_Fortran_binding.c and create inline code in the
compiler itself, i.e. in gcc/fortran/trans-expr.c's
gfc_conv_gfc_desc_to_cfi_desc and in gcc/fortran/trans-decl.c's
convert_CFI_desc.

The code in libgfortran is rather simple - and in the compiler itself,
knowledge about the type can be used, avoiding both type alias issues and
generating proper code – it also improves the performance.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug fortran/101635] FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
  2021-07-27  8:34 [Bug fortran/101635] New: FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc burnus at gcc dot gnu.org
@ 2021-07-27 13:29 ` pault at gcc dot gnu.org
  2021-10-18  8:29 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: pault at gcc dot gnu.org @ 2021-07-27 13:29 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101635

Paul Thomas <pault at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-07-27
     Ever confirmed|0                           |1
                 CC|                            |pault at gcc dot gnu.org

--- Comment #1 from Paul Thomas <pault at gcc dot gnu.org> ---

> Solution:  I think the proper fix is to remove both 
> _gfortran_gfc_desc_to_cfi_desc  and  _gfortran_cfi_desc_to_gfc_desc from
> libgfortran/runtime/ISO_Fortran_binding.c and create inline code in the
> compiler itself, i.e. in gcc/fortran/trans-expr.c's
> gfc_conv_gfc_desc_to_cfi_desc and in gcc/fortran/trans-decl.c's
> convert_CFI_desc.
> 
> The code in libgfortran is rather simple - and in the compiler itself,
> knowledge about the type can be used, avoiding both type alias issues and
> generating proper code – it also improves the performance.

I agree completely. The library code was used as a temporary measure since it
is easily tested outside the compiler. I think that the time has indeed come to
convert it to inline code.

I am still out of commission for gfortran work and so I won't take it yet.

Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug fortran/101635] FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
  2021-07-27  8:34 [Bug fortran/101635] New: FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc burnus at gcc dot gnu.org
  2021-07-27 13:29 ` [Bug fortran/101635] " pault at gcc dot gnu.org
@ 2021-10-18  8:29 ` cvs-commit at gcc dot gnu.org
  2021-10-19 10:06 ` burnus at gcc dot gnu.org
  2022-01-09 23:24 ` pinskia at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-10-18  8:29 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101635

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:64f9623765da3306b0ab6a47997dc5d62c2ea261

commit r12-4467-g64f9623765da3306b0ab6a47997dc5d62c2ea261
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Mon Oct 18 09:51:36 2021 +0200

    Fortran: Fix Bind(C) Array-Descriptor Conversion

    gfortran uses internally a different array descriptor ("gfc") as
    Fortran 2018 alias TS291113 defines for C interoperability via
    ISO_Fortran_binding.h ("CFI").  Hence, when calling a C function
    from Fortran, it has to be converted in the callee - and if a
    BIND(C) procedure is written in Fortran, the CFI argument has
    to be converted to gfc in order work with the rest of the FE
    code and the library calls.

    Before this patch, part was handled in the FE generated code and
    other parts in libgfortran.  With this patch, all code is generated
    and CFI is defined as proper type - visible in the debugger and to
    the middle end - avoiding both alias issues and missed optimization
    issues.

    This patch also fixes issues like: intent(out) deallocation in
    the bind(C) callee, using the CFI descriptor also for allocatable
    and pointer scalars and for len=* character strings.
    For 'select rank', it also optimizes the code + avoid accessing
    uninitialized memory if the dummy argument is allocatable/a pointer.
    It additionally rejects passing a descriptorless type(*) to an
    assumed-rank dummy argument. [F2018:C711]

            PR fortran/102086
            PR fortran/92189
            PR fortran/92621
            PR fortran/101308
            PR fortran/101309
            PR fortran/101635
            PR fortran/92482

    gcc/fortran/ChangeLog:

            * decl.c (gfc_verify_c_interop_param): Remove 'sorry' for
            scalar allocatable/pointer and len=*.
            * expr.c (is_CFI_desc): Return true for for those.
            * gfortran.h (CFI_type_kind_shift, CFI_type_mask,
            CFI_type_from_type_kind, CFI_VERSION, CFI_MAX_RANK,
            CFI_attribute_pointer, CFI_attribute_allocatable,
            CFI_attribute_other, CFI_type_Integer, CFI_type_Logical,
            CFI_type_Real, CFI_type_Complex, CFI_type_Character,
            CFI_type_ucs4_char, CFI_type_struct, CFI_type_cptr,
            CFI_type_cfunptr, CFI_type_other): New #define.
            * trans-array.c (CFI_FIELD_BASE_ADDR, CFI_FIELD_ELEM_LEN,
            CFI_FIELD_VERSION, CFI_FIELD_RANK, CFI_FIELD_ATTRIBUTE,
            CFI_FIELD_TYPE, CFI_FIELD_DIM, CFI_DIM_FIELD_LOWER_BOUND,
            CFI_DIM_FIELD_EXTENT, CFI_DIM_FIELD_SM,
            gfc_get_cfi_descriptor_field, gfc_get_cfi_desc_base_addr,
            gfc_get_cfi_desc_elem_len, gfc_get_cfi_desc_version,
            gfc_get_cfi_desc_rank, gfc_get_cfi_desc_type,
            gfc_get_cfi_desc_attribute, gfc_get_cfi_dim_item,
            gfc_get_cfi_dim_lbound, gfc_get_cfi_dim_extent,
gfc_get_cfi_dim_sm):
            New define/functions to access the CFI array descriptor.
            (gfc_conv_descriptor_type): New function for the GFC descriptor.
            (gfc_get_array_span): Handle expr of CFI descriptors and
            assumed-type descriptors.
            (gfc_trans_array_bounds): Remove 'static'.
            (gfc_conv_expr_descriptor): For assumed type, use the dtype of
            the actual argument.
            (structure_alloc_comps): Remove ' ' inside tabs.
            * trans-array.h (gfc_trans_array_bounds, gfc_conv_descriptor_type,
            gfc_get_cfi_desc_base_addr, gfc_get_cfi_desc_elem_len,
            gfc_get_cfi_desc_version, gfc_get_cfi_desc_rank,
            gfc_get_cfi_desc_type, gfc_get_cfi_desc_attribute,
            gfc_get_cfi_dim_lbound, gfc_get_cfi_dim_extent,
gfc_get_cfi_dim_sm):
            New prototypes.
            * trans-decl.c (gfor_fndecl_cfi_to_gfc, gfor_fndecl_gfc_to_cfi):
            Remove global vars.
            (gfc_build_builtin_function_decls): Remove their initialization.
            (gfc_get_symbol_decl, create_function_arglist,
            gfc_trans_deferred_vars): Update for CFI.
            (convert_CFI_desc): Remove and replace by ...
            (gfc_conv_cfi_to_gfc): ... this function
            (gfc_generate_function_code): Call it; create local GFC var for
CFI.
            * trans-expr.c (gfc_maybe_dereference_var): Handle CFI.
            (gfc_conv_subref_array_arg): Handle the if-noncontigous-only copy
in
            when the result should be a descriptor.
            (gfc_conv_gfc_desc_to_cfi_desc): Completely rewritten.
            (gfc_conv_procedure_call): CFI fixes.
            * trans-openmp.c (gfc_omp_is_optional_argument,
            gfc_omp_check_optional_argument): Handle optional
            CFI.
            * trans-stmt.c (gfc_trans_select_rank_cases): Cleanup, avoid
invalid
            code for allocatable/pointer dummies, which cannot be assumed size.
            * trans-types.c (gfc_cfi_descriptor_base): New global var.
            (gfc_get_dtype_rank_type): Skip rank init for rank < 0.
            (gfc_sym_type): Handle CFI dummies.
            (gfc_get_function_type): Update call.
            (gfc_get_cfi_dim_type, gfc_get_cfi_type): New.
            * trans-types.h (gfc_sym_type): Update prototype.
            (gfc_get_cfi_type): New prototype.
            * trans.c (gfc_trans_runtime_check): Make conditions more
consistent
            to avoid '<logical> AND_THEN <long int>' in conditions.
            * trans.h (gfor_fndecl_cfi_to_gfc, gfor_fndecl_gfc_to_cfi): Remove
            global-var declaration.

    libgfortran/ChangeLog:

            * ISO_Fortran_binding.h (CFI_type_cfunptr): Make unique type again.
            * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc,
            gfc_desc_to_cfi_desc): Add comment that those are no longer called
            by new code.

    libgomp/ChangeLog:

            * testsuite/libgomp.fortran/optional-bind-c.f90: New test.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/ISO_Fortran_binding_4.f90: Extend testcase.
            * gfortran.dg/PR100914.f90: Remove xfail.
            * gfortran.dg/PR100915.c: Expect CFI_type_cfunptr.
            * gfortran.dg/PR100915.f90: Handle CFI_type_cfunptr !=
CFI_type_cptr.
            * gfortran.dg/PR93963.f90: Extend select-rank tests.
            * gfortran.dg/bind-c-intent-out.f90: Change to dg-do run,
            update scan-dump.
            * gfortran.dg/bind_c_array_params_2.f90: Update/extend scan-dump.
            * gfortran.dg/bind_c_char_10.f90: Update scan-dump.
            * gfortran.dg/bind_c_char_8.f90: Remove dg-error "sorry".
            * gfortran.dg/c-interop/allocatable-dummy.f90: Remove xfail.
            * gfortran.dg/c-interop/c1255-1.f90: Likewise.
            * gfortran.dg/c-interop/c407c-1.f90: Update dg-error.
            * gfortran.dg/c-interop/cf-descriptor-5.f90: Remove xfail.
            * gfortran.dg/c-interop/cf-out-descriptor-3.f90: Likewise.
            * gfortran.dg/c-interop/cf-out-descriptor-4.f90: Likewise.
            * gfortran.dg/c-interop/cf-out-descriptor-5.f90: Likewise.
            * gfortran.dg/c-interop/contiguous-2.f90: Likewise.
            * gfortran.dg/c-interop/contiguous-3.f90: Likewise.
            * gfortran.dg/c-interop/deferred-character-1.f90: Likewise.
            * gfortran.dg/c-interop/deferred-character-2.f90: Likewise.
            * gfortran.dg/c-interop/fc-descriptor-3.f90: Likewise.
            * gfortran.dg/c-interop/fc-descriptor-5.f90: Likewise.
            * gfortran.dg/c-interop/fc-descriptor-6.f90: Likewise.
            * gfortran.dg/c-interop/fc-out-descriptor-3.f90: Likewise.
            * gfortran.dg/c-interop/fc-out-descriptor-4.f90: Likewise.
            * gfortran.dg/c-interop/fc-out-descriptor-5.f90: Likewise.
            * gfortran.dg/c-interop/fc-out-descriptor-6.f90: Likewise.
            * gfortran.dg/c-interop/ff-descriptor-5.f90: Likewise.
            * gfortran.dg/c-interop/ff-descriptor-6.f90: Likewise.
            * gfortran.dg/c-interop/fc-descriptor-7.f90: Remove xfail + extend.
            * gfortran.dg/c-interop/fc-descriptor-7-c.c: Update for changes.
            * gfortran.dg/c-interop/shape.f90: Add implicit none.
            * gfortran.dg/c-interop/typecodes-array-char-c.c: Add kind=4 char.
            * gfortran.dg/c-interop/typecodes-array-char.f90: Likewise.
            * gfortran.dg/c-interop/typecodes-array-float128.f90: Remove xfail.
            * gfortran.dg/c-interop/typecodes-scalar-basic.f90: Likewise.
            * gfortran.dg/c-interop/typecodes-scalar-float128.f90: Likewise.
            * gfortran.dg/c-interop/typecodes-scalar-int128.f90: Likewise.
            * gfortran.dg/c-interop/typecodes-scalar-longdouble.f90: Likewise.
            * gfortran.dg/iso_c_binding_char_1.f90: Remove dg-error "sorry".
            * gfortran.dg/pr93792.f90: Turn XFAIL into PASS.
            * gfortran.dg/ISO_Fortran_binding_19.f90: New test.
            * gfortran.dg/assumed_type_12.f90: New test.
            * gfortran.dg/assumed_type_13.c: New test.
            * gfortran.dg/assumed_type_13.f90: New test.
            * gfortran.dg/bind-c-char-descr.f90: New test.
            * gfortran.dg/bind-c-contiguous-1.c: New test.
            * gfortran.dg/bind-c-contiguous-1.f90: New test.
            * gfortran.dg/bind-c-contiguous-2.f90: New test.
            * gfortran.dg/bind-c-contiguous-3.c: New test.
            * gfortran.dg/bind-c-contiguous-3.f90: New test.
            * gfortran.dg/bind-c-contiguous-4.c: New test.
            * gfortran.dg/bind-c-contiguous-4.f90: New test.
            * gfortran.dg/bind-c-contiguous-5.c: New test.
            * gfortran.dg/bind-c-contiguous-5.f90: New test.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug fortran/101635] FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
  2021-07-27  8:34 [Bug fortran/101635] New: FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc burnus at gcc dot gnu.org
  2021-07-27 13:29 ` [Bug fortran/101635] " pault at gcc dot gnu.org
  2021-10-18  8:29 ` cvs-commit at gcc dot gnu.org
@ 2021-10-19 10:06 ` burnus at gcc dot gnu.org
  2022-01-09 23:24 ` pinskia at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: burnus at gcc dot gnu.org @ 2021-10-19 10:06 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101635

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
FIXED in GCC 12.

If new issues are found, please open an new PR and CC me.
(Like: PR 102815 - which exposes a big-endian issue with a testcase.)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug fortran/101635] FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
  2021-07-27  8:34 [Bug fortran/101635] New: FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc burnus at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2021-10-19 10:06 ` burnus at gcc dot gnu.org
@ 2022-01-09 23:24 ` pinskia at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-01-09 23:24 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101635

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-01-09 23:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27  8:34 [Bug fortran/101635] New: FAIL: gfortran.dg/PR93963.f90 – alias-handling issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc burnus at gcc dot gnu.org
2021-07-27 13:29 ` [Bug fortran/101635] " pault at gcc dot gnu.org
2021-10-18  8:29 ` cvs-commit at gcc dot gnu.org
2021-10-19 10:06 ` burnus at gcc dot gnu.org
2022-01-09 23:24 ` pinskia at gcc dot gnu.org

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).