Re: [Patch] PR fortran/92470 Fixes for CFI_address

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] PR fortran/92470 Fixes for CFI_address

Tobias Burnus-3
Hi all,

On 11/12/19 3:42 PM, Tobias Burnus wrote:
> (2) CFI_establish: For allocatables, it is clear – base_addr == NULL.
> For pointers, it is clear as well – it has to be '0' according to the
> standard. But for CFI_attribute_other … I have now asked at
> https://mailman.j3-fortran.org/pipermail/j3/2019-November/thread.html#11740 
>

While I still have problems to decipher the standard, regarding
CFI_establish, Steve L wrote:

"In the C descriptor world, arrays start at zero as they do in C. The
only way they can become non-zero is through argument association,
allocation or pointer association as specified in 18.5.3p3. For
non-pointer, not-allocatable objects (this means "other"), the lower
bounds are supposed to be always zero."

Hence, I now also set it for CFI_attribute_other to 0 – and check it in
a test case (most users there have NULL as base_addr, hence, only a
single assert is in that file).

Build on x86-64_gnu-linux.
OK for the trunk and GCC-9?

Tobias


establish-fix.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] PR fortran/92470 Fixes for CFI_address

Jakub Jelinek
On Tue, Nov 12, 2019 at 03:42:23PM +0100, Tobias Burnus wrote:

> 2019-12-11  Tobias Burnus  <[hidden email]>
>
> libgfortran/
> PR fortran/92470
> * runtime/ISO_Fortran_binding.c (CFI_address): Handle non-zero
> lower_bound; update error message.
> (CFI_allocate): Fix comment typo.
> (CFI_establish): Fix identation, fix typos, don't check values of 'dv'
> argument.
>
> gcc/testsuite/
> PR fortran/92470
> * gfortran.dg/ISO_Fortran_binding_17.c: New.
> * gfortran.dg/ISO_Fortran_binding_17.f90: New.
> * gfortran.dg/ISO_Fortran_binding_1.c (elemental_mult_c, allocate_c,
> section_c, select_part_c): Update for CFI_{address} changes;
> add asserts.

This broke on hosts where the system compiler doesn't have
ISO_Fortran_binding.h header installed (e.g. GCC 8 and earlier).

Fixed thusly, tested on x86_64-linux, committed to trunk as obvious.

2019-11-14  Jakub Jelinek  <[hidden email]>

        * gfortran.dg/ISO_Fortran_binding_17.c: Include
        ../../../libgfortran/ISO_Fortran_binding.h rather than
        ISO_Fortran_binding.h.

--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_17.c.jj 2019-11-13 10:54:37.081172852 +0100
+++ gcc/testsuite/gfortran.dg/ISO_Fortran_binding_17.c 2019-11-14 01:19:36.704285484 +0100
@@ -2,7 +2,7 @@
 
 #include <stdio.h>
 #include <assert.h>
-#include "ISO_Fortran_binding.h"
+#include "../../../libgfortran/ISO_Fortran_binding.h"
 
 void Csub(const CFI_cdesc_t *, size_t, CFI_index_t invalid);
 


        Jakub

Reply | Threaded
Open this post in threaded view
|

Re: [Patch] PR fortran/92470 Fixes for CFI_address

Jakub Jelinek
On Thu, Nov 14, 2019 at 09:30:42AM +0100, Andreas Schwab wrote:

> On Nov 14 2019, Jakub Jelinek wrote:
>
> > --- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_17.c.jj 2019-11-13 10:54:37.081172852 +0100
> > +++ gcc/testsuite/gfortran.dg/ISO_Fortran_binding_17.c 2019-11-14 01:19:36.704285484 +0100
> > @@ -2,7 +2,7 @@
> >  
> >  #include <stdio.h>
> >  #include <assert.h>
> > -#include "ISO_Fortran_binding.h"
> > +#include "../../../libgfortran/ISO_Fortran_binding.h"
>
> Shoudn't that be fixed generically, by directing the compiler to use the
> uninstalled headers?

Maybem it just needs somebody to spend the time and do the header discovery
in gfortran.dg.
The above is what is used in other testcases.

        Jakub

Reply | Threaded
Open this post in threaded view
|

Re: [Patch] PR fortran/92470 Fixes for CFI_address

Tobias Burnus-3
In reply to this post by Jakub Jelinek
On 11/14/19 1:31 AM, Jakub Jelinek wrote:
> This broke on hosts where the system compiler doesn't have
> ISO_Fortran_binding.h header installed (e.g. GCC 8 and earlier).

Aha, that's the reason.

> Fixed thusly, tested on x86_64-linux, committed to trunk as obvious.
> 2019-11-14  Jakub Jelinek  <[hidden email]>
>
> * gfortran.dg/ISO_Fortran_binding_17.c: Include
> ../../../libgfortran/ISO_Fortran_binding.h rather than
> ISO_Fortran_binding.h.

Thanks, Jakub, for the fix and sorry all for the breakage. I try to
remember in order to not do repeat this mistake :-/

Likewise fixed on GCC 9 as attached.

Tobias



committed.diff (1K) Download Attachment