[Patch] Fortran: dump-parse-tree.c fixes for OpenMP
I recently saw that 'ancestor:' wasn't handled in -fdump-parse-tree.
I also did run once into an ICE for the swap flag in atomics when
dumping. – This fixes the two.
Additionally, I changed 'ancestor' to a bit field.
Comments? If not, I will commit it later today.
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: dump-parse-tree.c fixes for OpenMP
gcc/fortran/ChangeLog:
* dump-parse-tree.c (show_omp_clauses): Handle ancestor modifier,
avoid ICE for GFC_OMP_ATOMIC_SWAP.
* gfortran.h (gfc_omp_clauses): Change 'anecestor' into a bitfield.
diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
index 64e04c043f6..14a307856fc 100644
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -1750,6 +1750,8 @@ show_omp_clauses (gfc_omp_clauses *omp_clauses)
if (omp_clauses->device)
{
fputs (" DEVICE(", dumpfile);
+ if (omp_clauses->ancestor)
+ fputs ("ANCESTOR:", dumpfile);
show_expr (omp_clauses->device);
fputc (')', dumpfile);
}
@@ -1894,7 +1896,7 @@ show_omp_clauses (gfc_omp_clauses *omp_clauses)
if (omp_clauses->atomic_op != GFC_OMP_ATOMIC_UNSET)
{
const char *atomic_op;
- switch (omp_clauses->atomic_op)
+ switch (omp_clauses->atomic_op & GFC_OMP_ATOMIC_MASK)
{
case GFC_OMP_ATOMIC_READ: atomic_op = "READ"; break;
case GFC_OMP_ATOMIC_WRITE: atomic_op = "WRITE"; break;
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index c25d1cca3a8..b2b0254a3c3 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1482,12 +1482,11 @@ typedef struct gfc_omp_clauses
struct gfc_expr *dist_chunk_size;
struct gfc_expr *message;
const char *critical_name;
- bool ancestor;
enum gfc_omp_default_sharing default_sharing;
enum gfc_omp_atomic_op atomic_op;
enum gfc_omp_defaultmap defaultmap[OMP_DEFAULTMAP_CAT_NUM];
int collapse, orderedc;
- unsigned nowait:1, ordered:1, untied:1, mergeable:1;
+ unsigned nowait:1, ordered:1, untied:1, mergeable:1, ancestor:1;
unsigned inbranch:1, notinbranch:1, nogroup:1;
unsigned sched_simd:1, sched_monotonic:1, sched_nonmonotonic:1;
unsigned simd:1, threads:1, depend_source:1, destroy:1, order_concurrent:1;
[Patch] [v3] Fortran: Fix Bind(C) Array-Descriptor Conversion (Move to Front-End Code)
Dear all,
a minor update [→ v3].
I searched for XFAIL in Sandra's c-interop/ and found
two remaining true** xfails, now fixed:
- gfortran.dg/c-interop/typecodes-scalar-basic.f90
The conversion of scalars of type(c_ptr) was mishandled;
fixed now; the fix did run into issues converting a string_cst,
which has also been fixed.
- gfortran.dg/c-interop/fc-descriptor-7.f90
this one uses TRANSPOSE which did not work [now mostly* does]
→ PR fortran/101309 now also fixed.
I forgot what the exact issue for the latter was. However, when
looking at the testcase and extending it, I did run into the
following issue - and at the end the testcase does now pass.
The issue I had was that when a contiguous check was requested
(i.e. only copy in when needed) it failed to work, when
parmse->expr was (a pointer to) a descriptor. I fixed that and
now most* things work.
OK for mainline? Comments? Suggestions? More PRs which fixes
this patch? Regressions? Test results?
Tobias
PS: I intent to commit this patch to the OG11 (devel/omp/gcc-11)
branch, in case someone wants to test it there.
PPS: Nice to have an extensive testcase suite - kudos to Sandra
in this case. I am sure Gerald will find more issues and once
it is in, I think I/we have to check some PRs + José's patches
whether for additional testcases + follow-up fixes.
(*) I write most as passing a (potentially) noncontiguous
assumed-rank array to a CONTIGUOUS assumed-rank array causes
an ICE as the scalarizer does not handle dynamic ranks alias
expr->rank == -1 / ss->dimen = -1.
I decided that that's a separate issue and filled:
https://gcc.gnu.org/PR102729
BTW, my impression is that fixing that PR might fix will solve
the trans*.c part of https://gcc.gnu.org/PR102641 - but I have
not investigated.
(**) There are still some 'xfail' in comments (outside dg-*)
whose tests now pass. And those where for two bugs in the same
statement, only one is reported - and the other only after fixing
the first one, which is fine.
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
[PATCH] PR fortran/102717 - ICE in gfc_simplify_reshape, at fortran/simplify.c:6843
Dear Fortranners,
when simplifying RESHAPE we hit a gcc_assert for negative entries in the
SHAPE array. Obvious solution: replace gcc_assert by an error message.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
As this is a safe fix, I'd like to backport to suitable branches.
Thanks,
Harald
Fortran: generate error message for negative elements in SHAPE array
gcc/fortran/ChangeLog:
PR fortran/102717
* simplify.c (gfc_simplify_reshape): Replace assert by error
message for negative elements in SHAPE array.
gcc/testsuite/ChangeLog:
PR fortran/102717
* gfortran.dg/reshape_shape_2.f90: New test.
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index f40e4930b58..d675f2c3aef 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -6840,7 +6840,13 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp,
gfc_extract_int (e, &shape[rank]);
gcc_assert (rank >= 0 && rank < GFC_MAX_DIMENSIONS);
- gcc_assert (shape[rank] >= 0);
+ if (shape[rank] < 0)
+ {
+ gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a "
+ "negative value %d for dimension %d",
+ &shape_exp->where, shape[rank], rank+1);
+ return &gfc_bad_expr;
+ }
rank++;
}
diff --git a/gcc/testsuite/gfortran.dg/reshape_shape_2.f90 b/gcc/testsuite/gfortran.dg/reshape_shape_2.f90
new file mode 100644
index 000..8f1757687bc
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/reshape_shape_2.f90
@@ -0,0 +1,7 @@
+! { dg-do compile }
+! PR fortran/102717
+
+program p
+ integer, parameter :: a(1) = 2
+ integer, parameter :: b(2) = reshape([3,4], -[a]) ! { dg-error "negative" }
+end
[PATCH] PR fortran/102716 - ICE in gfc_validate_kind(): Got bad kind
Dear Fortranners,
another simple and obvious fix: we need to reorder the argument checks
to the SHAPE intrinsic so that invalid KIND arguments can be detected.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
As I consider this a safe fix, I'd like to backport to suitable branches.
Thanks,
Harald
Fortran: fix order of checks for the SHAPE intrinsic
gcc/fortran/ChangeLog:
PR fortran/102716
* check.c (gfc_check_shape): Reorder checks so that invalid KIND
arguments can be detected.
gcc/testsuite/ChangeLog:
PR fortran/102716
* gfortran.dg/shape_10.f90: New test.
diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index 677209ee95e..cfaf9d26bbc 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -5086,6 +5086,13 @@ gfc_check_shape (gfc_expr *source, gfc_expr *kind)
if (gfc_invalid_null_arg (source))
return false;
+ if (!kind_check (kind, 1, BT_INTEGER))
+return false;
+ if (kind && !gfc_notify_std (GFC_STD_F2003, "%qs intrinsic "
+ "with KIND argument at %L",
+ gfc_current_intrinsic, &kind->where))
+return false;
+
if (source->rank == 0 || source->expr_type != EXPR_VARIABLE)
return true;
@@ -5098,13 +5105,6 @@ gfc_check_shape (gfc_expr *source, gfc_expr *kind)
return false;
}
- if (!kind_check (kind, 1, BT_INTEGER))
-return false;
- if (kind && !gfc_notify_std (GFC_STD_F2003, "%qs intrinsic "
- "with KIND argument at %L",
- gfc_current_intrinsic, &kind->where))
-return false;
-
return true;
}
diff --git a/gcc/testsuite/gfortran.dg/shape_10.f90 b/gcc/testsuite/gfortran.dg/shape_10.f90
new file mode 100644
index 000..4943c21b1d2
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/shape_10.f90
@@ -0,0 +1,6 @@
+! { dg-do compile }
+! PR fortran/102716
+
+program p
+ integer, parameter :: a(1) = shape([2], [1]) ! { dg-error "must be a scalar" }
+end
Re: [Patch] [v3] Fortran: Fix Bind(C) Array-Descriptor Conversion (Move to Front-End Code)
Hi Tobias, Am 13.10.21 um 18:01 schrieb Tobias Burnus: Dear all, a minor update [→ v3]. this has become an impressive work. I searched for XFAIL in Sandra's c-interop/ and found two remaining true** xfails, now fixed: - gfortran.dg/c-interop/typecodes-scalar-basic.f90 The conversion of scalars of type(c_ptr) was mishandled; fixed now; the fix did run into issues converting a string_cst, which has also been fixed. - gfortran.dg/c-interop/fc-descriptor-7.f90 this one uses TRANSPOSE which did not work [now mostly* does] → PR fortran/101309 now also fixed. I forgot what the exact issue for the latter was. However, when looking at the testcase and extending it, I did run into the following issue - and at the end the testcase does now pass. The issue I had was that when a contiguous check was requested (i.e. only copy in when needed) it failed to work, when parmse->expr was (a pointer to) a descriptor. I fixed that and now most* things work. OK for mainline? Comments? Suggestions? More PRs which fixes this patch? Regressions? Test results? Doesn't break my own codes so far. If nobody else responds within the next days, assume an OK from my side. This will also provide Gerhard with a new playground. ;-) Thanks for the patch! Harald Tobias PS: I intent to commit this patch to the OG11 (devel/omp/gcc-11) branch, in case someone wants to test it there. PPS: Nice to have an extensive testcase suite - kudos to Sandra in this case. I am sure Gerald will find more issues and once it is in, I think I/we have to check some PRs + José's patches whether for additional testcases + follow-up fixes. (*) I write most as passing a (potentially) noncontiguous assumed-rank array to a CONTIGUOUS assumed-rank array causes an ICE as the scalarizer does not handle dynamic ranks alias expr->rank == -1 / ss->dimen = -1. I decided that that's a separate issue and filled: https://gcc.gnu.org/PR102729 BTW, my impression is that fixing that PR might fix will solve the trans*.c part of https://gcc.gnu.org/PR102641 - but I have not investigated. (**) There are still some 'xfail' in comments (outside dg-*) whose tests now pass. And those where for two bugs in the same statement, only one is reported - and the other only after fixing the first one, which is fine. 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/g
Re: [PATCH] PR fortran/102717 - ICE in gfc_simplify_reshape, at fortran/simplify.c:6843
H Harald, when simplifying RESHAPE we hit a gcc_assert for negative entries in the SHAPE array. Obvious solution: replace gcc_assert by an error message. Regtested on x86_64-pc-linux-gnu. OK for mainline? As this is a safe fix, I'd like to backport to suitable branches. OK for both. Thanks for the patch! Best regards Thomas
Re: [PATCH] PR fortran/102716 - ICE in gfc_validate_kind(): Got bad kind
Hi Harald, another simple and obvious fix: we need to reorder the argument checks to the SHAPE intrinsic so that invalid KIND arguments can be detected. Regtested on x86_64-pc-linux-gnu. OK for mainline? As I consider this a safe fix, I'd like to backport to suitable branches. Also OK for both. Thanks a lot for the patch! Best regards Thomas
