committed – Re: [Patch] testsuite/lib/gfortran.exp: Add -I for ISO*.h [PR101305, PR101660] (was: Re: [Patch] gfortran.dg/dg.exp: Add libgfortran as -I flag for ISO*.h [PR101305] (was: [PATCH 3/3] [PR

[Tobias Burnus]

Now committed as obvious as 
https://gcc.gnu.org/r12-2808-g527a1cf32c27a3fbeaf6be7596241570d864cc4c

Follow-up suggestions are welcome. To recap, it changes three things:

* In the testcases, update "..." to "<...>" for the include
* -I $specpath/libgfortran - to find the .h file in the build dir (for 
in-build-tree testing)
* Update GFORTRAN_UNDER_TEST

The first two are very obvious, the latter applies, when running DejaGNU with, 
e.g.,
--target_board=unix\{-m32,-m64\}".
Those two multilib configs use different libgfortran build dirs and, hence,
the $specpath/libgfortran differs (e.g. x86.../libgfortran + 
x86.../32/libgfortran).

That's fine, except that when gfortran_init sets GFORTRAN_UNDER_TEST. That only
happend when it was unset. That's fine, except for multilib test runs, it is not
updated when gfortran_init is called for the next multilib run. Thus, in that 
case
the previous settings are used. – For the discussion in this thread, this means 
the
wrong ISO_Fortran_binding.h is read.

Or in previous wording:

On 29.07.21 11:51, Tobias Burnus wrote:

On 29.07.21 09:09, Jakub Jelinek wrote:

On Thu, Jul 29, 2021 at 12:56:32AM +0200, Jakub Jelinek wrote:

On Wed, Jul 28, 2021 at 01:22:53PM +0200, Tobias Burnus wrote:

gfortran.dg/dg.exp: Add libgfortran as -I flag for ISO*.h [PR101305]

Wouldn't it be better to do that in gcc/testsuite/lib/gfortran.exp
to GFORTRAN_UNDER_TEST there next to
-B$specpath/libgfortran/ ?


I guess so – and that's what I did. However, I had to ensure that it
gets reset otherwise it picks up the wrong header in multilib runs;
this also affects the -B$specpath/libgfortran bit, but I think that
makes sense.


Though, I guess we need that mostly for the C FE, so perhaps it needs
to go
at the start of additional_flags=, whether TEST_ALWAYS_FLAGS is empty or
not.


For the main testsuite (gcc/testsuite/*fortran*/), I believe the patch
above is sufficient as everything runs through GFORTRAN_UNDER_TEST.

I am also inclined not to add flags to TEST_ALWAYS_FLAGS which then
might get applied to other/pure C/C++ tests.

Regarding libgomp: that one uses xgcc for the compilation. I don't
really see a need to use the Fortran array descriptor from a C program
in libgomp's testsuite. Thus, I am inclined to ignore libgomp.
Otherwise, as libgomp does not gfortran_init and handles libraries
separately, I think the code needs to be put into
libgomp.*fortran/fortran.exp.

Thoughts? Okay?


Tobias

-
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
commit 527a1cf32c27a3fbeaf6be7596241570d864cc4c
Author: Tobias Burnus 
Date:   Mon Aug 9 12:35:23 2021 +0200

testsuite/lib/gfortran.exp: Add -I for ISO*.h [PR101305, PR101660]

This patch adds -I$specdir/libgfortran to GFORTRAN_UNDER_TEST, when
set by proc gfortran_init. As the $specdir depends on the multilib
setting, it has to be re-set for a different multilib; hence, we track
whether a previous call to gfortran_init set that var or whether it
was set differently.

gcc/testsuite/
PR libfortran/101305
PR fortran/101660

* lib/gfortran.exp (gfortran_init): Add -I $specdir/libgfortran to
GFORTRAN_UNDER_TEST; update it when set by previous gfortran_init call.
* gfortran.dg/ISO_Fortran_binding_1.c: Use <...> not "..." for
ISO_Fortran_binding.h's #include.
* gfortran.dg/ISO_Fortran_binding_10.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_11.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_12.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_15.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_16.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_17.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_18.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_3.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_5.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_6.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_7.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_8.c: Likewise.
* gfortran.dg/ISO_Fortran_binding_9.c: Likewise.
* gfortran.dg/PR94327.c: Likewise.
* gfortran.dg/PR94331.c: Likewise.
* gfortran.dg/bind_c_array_params_3_aux.c: Likewise.
* gfortran.dg/iso_fortran_binding_uint8_array_driver.c: Likewise.
* gfortran.dg/pr93524.c: Likewise.
---
 gcc/testsuite/gfortran.dg/ISO_Fortran_binding_1.c  |  2 +-
 gcc/testsuite/gfortran.dg/ISO_Fortran_binding_10.c |  2 +-
 gcc/testsuite/gfortran.dg/ISO_Fortran_binding_11.c |  2 +-
 gcc/testsuite/gfortran.dg/ISO_Fortran_binding_12.c |  2 +-
 gcc/testsuite/gfortran.dg/ISO

Re: [PATCH 3/3] [PR libfortran/101305] Fix ISO_Fortran_binding.h paths in gfortran testsuite

[Tobias Burnus]

Hi Andreas,

On 04.08.21 11:00, Andreas Schwab wrote:

On Jul 13 2021, Sandra Loosemore wrote:

-#include "../../../libgfortran/ISO_Fortran_binding.h"
+#include "ISO_Fortran_binding.h"

Shouldn't that use  since that is an installed
header, not one that is supposed to be picked up from the current
directory?


Yes – thus, it was changed by my patch:
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/576970.html

https://gcc.gnu.org/r12-2808-g527a1cf32c27a3fbeaf6be7596241570d864cc4c  


Thanks for keeping an eye on patches and reporting issues :-)

Tobias

-
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

Re: [Patch v2 Fortran] Fix c_float128 and c_float128_complex on targets with 128-bit long double.

[Sandra Loosemore]

On 8/5/21 2:09 PM, Michael Meissner wrote:


All PowerPC systems that I'm aware of that use 128-bit floating point use the
IBM format.  It is anticipated that one or more Linux distributions in the
future may move to IEEE 128-bit format, but right now, I'm not aware that any
have moved.


OK.  I now think it's correct that the Fortran front end doesn't support 
c_float128 and c_float128_complex on this target, but that the code that 
initializes those constants is still buggy.  The reason why it shouldn't 
support them is that all 3 128-bit floating-point modes on PowerPC would 
map onto kind=16, and we can only support one of them unless we make 
some exception to the formula for mapping precision -> kind.  And the 
mode the Fortran front end already prefers is the one that corresponds 
to long double or TFmode.


So the reason why the current code is wrong is that 
gfc_float128_type_node only gets set if there is a type with 128-bit 
precision that is not long double, so c_float128 wouldn't be supported 
on a target where __float128 is equivalent to long double.  Moreover, on 
targets where it does define gfc_float128_type_node, it's not doing 
anything to ensure that it's actually the same IEEE representation as 
__float128.  Both problems can be fixed by just using float128_type_node 
to get the type corresponding to __float128.  If that's not a mode the 
Fortran front end knows about, get_real_kind_from_node will detect that.


I suspect there are some similar lurking bugs in other uses of 
gfc_float128_type_node in the Fortran front end, but that's out of scope 
for my current project focusing on interoperability features.  I'll file 
an issue about it, though.


Here's a revised patch.  Fortran maintainers, is this one OK to check in?

-Sandra
commit 073dd403d11553c199610b038285b203c130cee5
Author: Sandra Loosemore 
Date:   Tue Aug 3 16:21:16 2021 -0700

Fix c_float128 and c_float128_complex definitions.

gfc_float128_type_node is only non-NULL on targets that support a
128-bit type that is not long double.  Use float128_type_node instead
when computing the value of the kind constants c_float128 and
c_float128_complex from the ISO_C_BINDING intrinsic module; this also
ensures it actually corresponds to __float128 (the IEEE encoding) and
not some other 128-bit floating-point type.

2021-08-09  Sandra Loosemore  

gcc/fortran/
	* iso-c-binding.def (c_float128, c_float128_complex): Check
	float128_type_node instead of gfc_float128_type_node.
	* trans-types.c (gfc_init_kinds, gfc_build_real_type):
	Update comments re supported 128-bit floating-point types.

diff --git a/gcc/fortran/iso-c-binding.def b/gcc/fortran/iso-c-binding.def
index 8bf69ef..e65c750 100644
--- a/gcc/fortran/iso-c-binding.def
+++ b/gcc/fortran/iso-c-binding.def
@@ -114,9 +114,14 @@ NAMED_REALCST (ISOCBINDING_DOUBLE, "c_double", \
get_real_kind_from_node (double_type_node), GFC_STD_F2003)
 NAMED_REALCST (ISOCBINDING_LONG_DOUBLE, "c_long_double", \
get_real_kind_from_node (long_double_type_node), GFC_STD_F2003)
+
+/* GNU Extension.  Note that the equivalence here is specifically to
+   the IEEE 128-bit type __float128; if that does not map onto a type
+   otherwise supported by the Fortran front end, get_real_kind_from_node
+   will reject it as unsupported.  */
 NAMED_REALCST (ISOCBINDING_FLOAT128, "c_float128", \
-	   gfc_float128_type_node == NULL_TREE \
-		  ? -4 : get_real_kind_from_node (gfc_float128_type_node), \
+		(float128_type_node == NULL_TREE \
+		 ? -4 : get_real_kind_from_node (float128_type_node)), \
 	   GFC_STD_GNU)
 NAMED_CMPXCST (ISOCBINDING_FLOAT_COMPLEX, "c_float_complex", \
get_real_kind_from_node (float_type_node), GFC_STD_F2003)
@@ -124,9 +129,11 @@ NAMED_CMPXCST (ISOCBINDING_DOUBLE_COMPLEX, "c_double_complex", \
get_real_kind_from_node (double_type_node), GFC_STD_F2003)
 NAMED_CMPXCST (ISOCBINDING_LONG_DOUBLE_COMPLEX, "c_long_double_complex", \
get_real_kind_from_node (long_double_type_node), GFC_STD_F2003)
+
+/* GNU Extension.  Similar issues to c_float128 above.  */
 NAMED_CMPXCST (ISOCBINDING_FLOAT128_COMPLEX, "c_float128_complex", \
-	   gfc_float128_type_node == NULL_TREE \
-		  ? -4 : get_real_kind_from_node (gfc_float128_type_node), \
+		(float128_type_node == NULL_TREE \
+		 ? -4 : get_real_kind_from_node (float128_type_node)), \
 	   GFC_STD_GNU)
 
 NAMED_LOGCST (ISOCBINDING_BOOL, "c_bool", \
diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
index 50fda43..8250219 100644
--- a/gcc/fortran/trans-types.c
+++ b/gcc/fortran/trans-types.c
@@ -446,7 +446,7 @@ gfc_init_kinds (void)
   if (!targetm.scalar_mode_supported_p (mode))
 	continue;
 
-  /* Only let float, double, long double and __float128 go through.
+  /* Only let float, double, long double and TFmode go through.
 	 Runtime support for others is not provided, s