PING – Re: [Patch] Fortran: Handle allocated() with coindexed scalars [PR93834] (was: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in trans_caf_is_present, at fortran/trans-intrinsic.c:8469)
Patch PING – see comment in the follow-up email of the patch email - and
in the email(s) before in that thread.
Tobias
On 07.09.21 16:33, Tobias Burnus wrote:
Now I actually tested the patch – and fixed some issues.
OK? – It does add support for 'allocated(a[i])' by treating
it as 'allocated(a)', as 'a' must be collectively allocated
("established") on all images of the team.*
'a[i]' is (probably) an allocatable, following Malcolm in
answer to my question to the J3-list as linked below.
Tobias
* Ignoring issues related to failed images. It could
also be handled by fetching 'a' from the remote
image, but I am not sure that's better in terms of
handling failed images.
PS:
On 07.09.21 10:02, Tobias Burnus wrote:
Hi Harald,
I spend yesterday about two hours with this. Now I am still
tired but understand more. I think the confusion between the
two of us is due to wording and in which directions the
thoughts then go:
Talking about coindexed, all of a[i], b[i]%c and c%d[i] are
coindexed and there are many constraints like "shall not be
a coindexed variable" – which then rejects all of those.
That's what I was thinking of.
I think your starting point is that while ('a' = allocatable)
a, b%a, c[5]%d(1)%a
are ALLOCATABLE, adding a subobject reference such as
a(:), b%a(:,:), c[5]%d(1)%a(:,:,:)
makes the variable no longer allocatable.
I think that's what you were thinking of.
We then both argued along those different lines – which caused
the confusion as we both thought we talked about the same.
While those cases are clear, the question is whether
a[i] or b%a[i]
is allocatable or not – assuming that 'a' is a scalar.
(For an array, '(:)' has to appear before the image-selector,
which in turn makes it nonallocatable.)
I tried to pinpoint the words for this in the standard – and
failed. I think I need a "how to read the Fortran standard" 101
and some long time actually reading it :-(
Malcolm has answered me – and he believes (but only offhand) that
a[i] and b%a[i]
_are_ allocatable. See (6) at
https://mailman.j3-fortran.org/pipermail/j3/2021-September/013322.html
This implies that
if ( allocated (a[i]) .and. allocated (b%a[i]) ) stop 1
is valid.
However, I do note that coarray allocatables have to be collectively
(de)allocated, therefore
allocated (a[i]) .and. allocated (b%a[i])
is equivalent to
allocated (a) .and. allocated (b%a)
at least assuming that no image has failed.
First: Does this answer all the questions you had and resolved the
confusion?
Secondly, do you agree about the last bits of the analysis?
Thirdly, what do you think of the attached patch?
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
[Patch] Fortran: Add gfc_simple_for_loop aux function
This patch adds a useful auxiliary function to generate a loop.
I intent to use it for:
(A) An updated/cleaned-up version of
"[Patch] Fortran: Fix Bind(C) Array-Descriptor Conversion (Move to Front-End
Code)"
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/578904.html
→ new function used four times
(B) For the SIZE handling part of
PR94070 - Assumed-rank arrays – bounds mishandled,
SIZE/SHAPE/UBOUND/LBOUND
which I am currently writing (→ used once)
The main reason of splitting this patch off is to permit to be able to
submit/commit
the two patches separately without having a code overlap.
However, in principle there is no reason that this patch cannot be reviewed
and/or
committed separately instead of in the same review and/or commit with one of
the other
patches.
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
Fortran: Add gfc_simple_for_loop aux function
Function to generate a simple loop (to be used internally).
Callers will be added in follow-up commits.
gcc/fortran/
* trans-expr.c (gfc_simple_for_loop): New.
* trans.h (gfc_simple_for_loop): New prototype.
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 18d665192f0..761f8c65234 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -11717,3 +11717,37 @@ gfc_trans_assign (gfc_code * code)
{
return gfc_trans_assignment (code->expr1, code->expr2, false, true);
}
+
+/* Generate a simple loop for internal use of the form
+ for (var = begin; var end; var += step)
+ body; */
+void
+gfc_simple_for_loop (stmtblock_t *block, tree var, tree begin, tree end,
+ enum tree_code cond, tree step, tree body)
+{
+ tree tmp;
+
+ /* var = begin. */
+ gfc_add_modify (block, var, begin);
+
+ /* Loop: for (var = begin; var end; var += step). */
+ tree label_loop = gfc_build_label_decl (NULL_TREE);
+ tree label_cond = gfc_build_label_decl (NULL_TREE);
+ TREE_USED (label_loop) = 1;
+ TREE_USED (label_cond) = 1;
+
+ gfc_add_expr_to_block (block, build1_v (GOTO_EXPR, label_cond));
+ gfc_add_expr_to_block (block, build1_v (LABEL_EXPR, label_loop));
+
+ /* Loop body. */
+ gfc_add_expr_to_block (block, body);
+
+ /* End of loop body. */
+ tmp = fold_build2_loc (input_location, PLUS_EXPR, TREE_TYPE (var), var, step);
+ gfc_add_modify (block, var, tmp);
+ gfc_add_expr_to_block (block, build1_v (LABEL_EXPR, label_cond));
+ tmp = fold_build2_loc (input_location, cond, boolean_type_node, var, end);
+ tmp = build3_v (COND_EXPR, tmp, build1_v (GOTO_EXPR, label_loop),
+ build_empty_stmt (input_location));
+ gfc_add_expr_to_block (block, tmp);
+}
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index 78578cfd732..1b622fc1f2e 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -518,6 +518,8 @@ tree gfc_string_to_single_character (tree len, tree str, int kind);
tree gfc_get_tree_for_caf_expr (gfc_expr *);
void gfc_get_caf_token_offset (gfc_se*, tree *, tree *, tree, tree, gfc_expr *);
tree gfc_caf_get_image_index (stmtblock_t *, gfc_expr *, tree);
+void gfc_simple_for_loop (stmtblock_t *, tree, tree, tree, enum tree_code, tree,
+ tree);
/* Find the decl containing the auxiliary variables for assigned variables. */
[PATCH, Fortran] Use _Float128 rather than __float128 for c_float128 kind
On 9/5/21 11:20 PM, Sandra Loosemore wrote:
Unless the aarch64 maintainers think it is a bug that __float128 is not
supported, I think the right solution here is the one I was thinking of
previously, to fix the Fortran front end to tie the C_FLOAT128 kind to
_Float128 rather than __float128, and fix the runtime support and test
cases accordingly. Then there should be no need to depend on quadmath.h
at all. C_FLOAT128 is a GNU extension and _Float128 is supported on a
superset of targets that __float128 is, so there should be no issue with
backward compatibility.
Here's a new patch that does this. I've tested it on x86_64-linux-gnu,
powerpc64le-linux-gnu, and aarch64-linux-gnu, and it does fix the
previously reported failure compiling gfortran.dg/PR100914.c on aarch64.
OK to commit?
-Sandra
commit cc7e47df550485654efa5f523c3be35007125340
Author: Sandra Loosemore
Date: Tue Sep 14 19:07:36 2021 -0700
Fortran: Use _Float128 rather than __float128 for c_float128 kind.
The GNU Fortran manual documents that the c_float128 kind corresponds
to __float128, but in fact the implementation uses float128_type_node,
which is _Float128. Both refer to the 128-bit IEEE/ISO encoding, but
some targets including aarch64 only define _Float128 and not __float128,
and do not provide quadmath.h. This caused errors in some test cases
referring to __float128.
This patch changes the documentation (including code comments) and
test cases to use _Float128 to match the implementation.
2021-09-16 Sandra Loosemore
gcc/fortran/
* intrinsic.texi (ISO_C_BINDING): Change C_FLOAT128 to correspond
to _Float128 rather than __float128.
* iso-c-binding.def (c_float128): Update comments.
* trans-intrinsic.c (gfc_builtin_decl_for_float_kind): Likewise.
(build_round_expr): Likewise.
(gfc_build_intrinsic_lib_fndcecls): Likewise.
* trans-types.h (gfc_real16_is_float128): Likewise.
gcc/testsuite/
* gfortran.dg/PR100914.c: Do not include quadmath.h. Use
_Float128 _Complex instead of __complex128.
* gfortran.dg/PR100914.f90: Add -Wno-pedantic to suppress error
about use of _Float128.
* gfortran.dg/c-interop/typecodes-array-float128-c.c: Use
_Float128 instead of __float128.
* gfortran.dg/c-interop/typecodes-sanity-c.c: Likewise.
* gfortran.dg/c-interop/typecodes-scalar-float128-c.c: Likewise.
* lib/target-supports.exp
(check_effective_target_fortran_real_c_float128): Update comments.
libgfortran/
* ISO_Fortran_binding.h: Update comments.
* runtime/ISO_Fortran_binding.c: Likewise.
diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi
index 1aacd33..1b9a89d 100644
--- a/gcc/fortran/intrinsic.texi
+++ b/gcc/fortran/intrinsic.texi
@@ -15193,8 +15193,8 @@ In addition to the integer named constants required by the Fortran 2003
standard and @code{C_PTRDIFF_T} of TS 29113, GNU Fortran provides as an
extension named constants for the 128-bit integer types supported by the
C compiler: @code{C_INT128_T, C_INT_LEAST128_T, C_INT_FAST128_T}.
-Furthermore, if @code{__float128} is supported in C, the named constants
-@code{C_FLOAT128, C_FLOAT128_COMPLEX} are defined.
+Furthermore, if @code{_Float128} is supported in C, the named constants
+@code{C_FLOAT128} and @code{C_FLOAT128_COMPLEX} are defined.
@multitable @columnfractions .15 .35 .35 .35
@headitem Fortran Type @tab Named constant @tab C type@tab Extension
@@ -15225,11 +15225,11 @@ Furthermore, if @code{__float128} is supported in C, the named constants
@item @code{REAL} @tab @code{C_FLOAT} @tab @code{float}
@item @code{REAL} @tab @code{C_DOUBLE}@tab @code{double}
@item @code{REAL} @tab @code{C_LONG_DOUBLE} @tab @code{long double}
-@item @code{REAL} @tab @code{C_FLOAT128} @tab @code{__float128}@tab Ext.
+@item @code{REAL} @tab @code{C_FLOAT128} @tab @code{_Float128}@tab Ext.
@item @code{COMPLEX}@tab @code{C_FLOAT_COMPLEX} @tab @code{float _Complex}
@item @code{COMPLEX}@tab @code{C_DOUBLE_COMPLEX}@tab @code{double _Complex}
@item @code{COMPLEX}@tab @code{C_LONG_DOUBLE_COMPLEX}@tab @code{long double _Complex}
-@item @code{REAL} @tab @code{C_FLOAT128_COMPLEX} @tab @code{__float128 _Complex} @tab Ext.
+@item @code{COMPLEX}@tab @code{C_FLOAT128_COMPLEX} @tab @code{_Float128 _Complex} @tab Ext.
@item @code{LOGICAL}@tab @code{C_BOOL} @tab @code{_Bool}
@item @code{CHARACTER}@tab @code{C_CHAR}@tab @code{char}
@end multitable
diff --git a/gcc/fortran/iso-c-binding.def b/gcc/fortran/iso-c-binding.def
index e65c750..50256fe 100644
--- a/gcc/fortran/iso-c-binding.def
+++ b/gcc/fortran/iso-c-binding.def
@@ -116,7 +116,7 @@ NAMED_REALCST (ISOCBINDING_LONG_DOUBLE, "c_long_double", \
get_real_kind_from_node (long_double_type_node
Re: [PATCH, Fortran] Revert to non-multilib-specific ISO_Fortran_binding.h
On Tue, 14 Sep 2021, Gerald Pfeifer wrote: >> And, related, does the following make sense and fixes the issue? >> >> --- a/libgfortran/ISO_Fortran_binding.h >> +++ b/libgfortran/ISO_Fortran_binding.h >> @@ -228,5 +228,5 @@ extern int CFI_setpointer (CFI_cdesc_t *, CFI_cdesc_t *, >> const CFI_index_t []); >> >> /* This is the 80-bit encoding on x86; Fortran assigns it kind 10. */ >> -#elif (LDBL_MANT_DIG == 64 \ >> +#elif ((LDBL_MANT_DIG == 64 || LDBL_MANT_DIG == 53) \ >> && LDBL_MIN_EXP == -16381 \ >> && LDBL_MAX_EXP == 16384) > Yes, with this patch (on top of current trunk) i586-freebsd-* is back > in bootstrap land. :) Neither this (which fixes the bootstrap) nor Sandra's rewrite (which does not, but seemed generally liked) has been committed. Can someone please push the former (and possibly later)? Thank you, Gerald
