Re: [PATCH 1/8] OpenMP: lvalue parsing for map/to/from clauses (C++)
Am 05.01.24 um 13:23 schrieb Julian Brown:
On Wed, 20 Dec 2023 15:31:15 +0100
Tobias Burnus wrote:
Here's a rebased/retested version which fixes those bits (I haven't
adjusted the libgomp.texi bit you noted yet, though).
How does this look now?
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -13499,7 +13499,11 @@ gimplify_adjust_omp_clauses_1 (splay_tree_node n, void
*data)
if (TREE_CODE (dtype) == REFERENCE_TYPE)
dtype = TREE_TYPE (dtype);
/* FIRSTPRIVATE_POINTER doesn't work well if we have a
-multiply-indirected pointer. */
+multiply-indirected pointer. If we have a reference to a pointer to
+a pointer, it's possible that this should really be
+GOMP_MAP_FIRSTPRIVATE_REFERENCE -- but that also doesn't work at the
+moment, so stick with this. (See testcase
+baseptrs-4.C:ref2ptrptr_offset_decl_member_slice). */
Looks as we should have a tracking PR about this; can you file one?
* * *
+ if (processing_template_decl)
+{
+ if (type_dependent_expression_p (array_expr)
+ || type_dependent_expression_p (index)
+ || type_dependent_expression_p (length))
+ return build_min_nt_loc (loc, OMP_ARRAY_SECTION, array_expr, index,
+length);
+}
I personally find it more readable if combined in a single 'if' condition.
+ /* Turn *foo into foo[0:1]. */
+ decl = TREE_OPERAND (decl, 0);
+ STRIP_NOPS (decl);
+
+ /* If we have "*foo" and
+- it's an indirection of a reference, "unconvert" it, i.e.
+ strip the indirection (to just "foo").
+- it's an indirection of a pointer, turn it into
+ "foo[0:1]". */
+ if (!ref_p)
+ decl = grok_omp_array_section (loc, decl, integer_zero_node,
+ integer_one_node);
I would remove the first comment and remove the two succeeding lines
below the second comment.
+ /* This code rewrites a parsed expression containing various tree
+codes used to represent array accesses into a more uniform nest of
+OMP_ARRAY_SECTION nodes before it is processed by
+semantics.cc:handle_omp_array_sections_1. It might be more
+efficient to move this logic to that function instead, analysing
+the parsed expression directly rather than this preprocessed
+form. */
Or to do this transformation in handle_omp_array_sections to get still a
unified result in the middle end. I see advantages of all three
solutions. (Doing this in parse.cc (as currently done) feels a bit odd,
though.)
* * *
build_omp_array_section (location_t loc, tree array_expr, tree index,
+tree length)
+{
+ tree idxtype;
+
+ /* If we know the integer bounds, create an index type with exact
+ low/high (or zero/length) bounds. Otherwise, create an incomplete
+ array type. (This mostly only affects diagnostics.) */
+ if (index != NULL_TREE
+ && length != NULL_TREE
+ && TREE_CODE (index) == INTEGER_CST
+ && TREE_CODE (length) == INTEGER_CST)
+{
+ tree low = fold_convert (sizetype, index);
+ tree high = fold_convert (sizetype, length);
+ high = size_binop (PLUS_EXPR, low, high);
+ high = size_binop (MINUS_EXPR, high, size_one_node);
+ idxtype = build_range_type (sizetype, low, high);
+}
+ else if ((index == NULL_TREE || integer_zerop (index))
+ && length != NULL_TREE
+ && TREE_CODE (length) == INTEGER_CST)
+idxtype = build_index_type (length);
+ else
+idxtype = NULL_TREE;
+
+ tree type = TREE_TYPE (array_expr);
+ gcc_assert (type);
+ type = non_reference (type);
+
+ tree sectype, eltype = TREE_TYPE (type);
+
+ /* It's not an array or pointer type. Just reuse the type of the
+ original expression as the type of the array section (an error will be
+ raised anyway, later). */
+ if (eltype == NULL_TREE)
+sectype = TREE_TYPE (array_expr);
+ else
+sectype = build_array_type (eltype, idxtype);
+
+ return build3_loc (loc, OMP_ARRAY_SECTION, sectype, array_expr, index,
+length);
+}
I wonder whether it would be more readable if one moves all the
'idxtype' handling into the last 'else' branch.
* * *
LGTM - please file the PR and consider the readability items above.
Thanks,
Tobias
[libgfortran, patch] PR113223 NAMELIST internal write missing leading blank character
Committed as simple and obvious. Initial patch thanks to Steve. When using git gcc-commit-mklog how does one add in the coauthor? The master branch has been updated by Jerry DeLisle : https://gcc.gnu.org/g:add995ec117d756e61d207041cd32f937c1a1cd9 commit r14-6986-gadd995ec117d756e61d207041cd32f937c1a1cd9 Author: Jerry DeLisle Date: Sun Jan 7 10:22:19 2024 -0800 libgfortran: Emit a space at beginning of internal unit NML. PR libgfortran/113223 libgfortran/ChangeLog: * io/write.c (namelist_write): If internal_unit precede with space. gcc/testsuite/ChangeLog: * gfortran.dg/dtio_25.f90: Update. * gfortran.dg/namelist_57.f90: Update. * gfortran.dg/namelist_65.f90: Update.
Re: [libgfortran, patch] PR113223 NAMELIST internal write missing leading blank character
Hi Jerry! On 1/7/24 19:40, Jerry D wrote: Committed as simple and obvious. Initial patch thanks to Steve. When using git gcc-commit-mklog how does one add in the coauthor? % git help gcc-commit-mklog ... --co CO Add Co-Authored-By trailer (comma separated) However, I usually add this line manually later. Regarding the format, have a look at existing log messages. Cheers, Harald The master branch has been updated by Jerry DeLisle : https://gcc.gnu.org/g:add995ec117d756e61d207041cd32f937c1a1cd9 commit r14-6986-gadd995ec117d756e61d207041cd32f937c1a1cd9 Author: Jerry DeLisle Date: Sun Jan 7 10:22:19 2024 -0800 libgfortran: Emit a space at beginning of internal unit NML. PR libgfortran/113223 libgfortran/ChangeLog: * io/write.c (namelist_write): If internal_unit precede with space. gcc/testsuite/ChangeLog: * gfortran.dg/dtio_25.f90: Update. * gfortran.dg/namelist_57.f90: Update. * gfortran.dg/namelist_65.f90: Update.
[PATCH] Fortran: SIZE optional DIM argument having OPTIONAL+VALUE attributes [PR113245]
Dear all,
the attached, actually rather obvious patch fixes an issue when
an optional dummy with the value attribute was passed as DIM
argument to the SIZE intrinsic. Instead of some hand-crafted,
incomplete presence check for the argument, it makes more sense
to rely on gfc_conv_expr_present().
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Thanks,
Harald
From 49f5c89f6bdddbb225ca70f8df78a75252b0b2d5 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Sun, 7 Jan 2024 22:24:25 +0100
Subject: [PATCH] Fortran: SIZE optional DIM argument having OPTIONAL+VALUE
attributes [PR113245]
gcc/fortran/ChangeLog:
PR fortran/113245
* trans-intrinsic.cc (gfc_conv_intrinsic_size): Use
gfc_conv_expr_present() for proper check of optional DIM argument.
gcc/testsuite/ChangeLog:
PR fortran/113245
* gfortran.dg/size_optional_dim_2.f90: New test.
---
gcc/fortran/trans-intrinsic.cc| 4 +--
.../gfortran.dg/size_optional_dim_2.f90 | 31 +++
2 files changed, 32 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/size_optional_dim_2.f90
diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index d973c49380c..74139262657 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -8025,9 +8025,6 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr)
argse.data_not_needed = 1;
gfc_conv_expr (&argse, actual->expr);
gfc_add_block_to_block (&se->pre, &argse.pre);
- cond = fold_build2_loc (input_location, NE_EXPR, logical_type_node,
- argse.expr, null_pointer_node);
- cond = gfc_evaluate_now (cond, &se->pre);
/* 'block2' contains the arg2 absent case, 'block' the arg2 present
case; size_var can be used in both blocks. */
tree size_var = gfc_create_var (TREE_TYPE (size), "size");
@@ -8038,6 +8035,7 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr)
tmp = fold_build2_loc (input_location, MODIFY_EXPR,
TREE_TYPE (size_var), size_var, size);
gfc_add_expr_to_block (&block2, tmp);
+ cond = gfc_conv_expr_present (actual->expr->symtree->n.sym);
tmp = build3_v (COND_EXPR, cond, gfc_finish_block (&block),
gfc_finish_block (&block2));
gfc_add_expr_to_block (&se->pre, tmp);
diff --git a/gcc/testsuite/gfortran.dg/size_optional_dim_2.f90 b/gcc/testsuite/gfortran.dg/size_optional_dim_2.f90
new file mode 100644
index 000..698702b0974
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/size_optional_dim_2.f90
@@ -0,0 +1,31 @@
+! { dg-do run }
+! { dg-additional-options "-fdump-tree-original" }
+! PR fortran/113245 - SIZE, optional DIM argument, w/ OPTIONAL+VALUE attributes
+
+program p
+ implicit none
+ real:: a(2,3)
+ integer :: expect
+ expect = size (a,2)
+ call ref (a,2)
+ call val (a,2)
+ expect = size (a)
+ call ref (a)
+ call val (a)
+contains
+ subroutine ref (x, dim)
+real, intent(in) :: x(:,:)
+integer, optional, intent(in) :: dim
+print *, "present(dim), size(a,dim) =", present (dim), size (x,dim=dim)
+if (size (x,dim=dim) /= expect) stop 1
+ end
+ subroutine val (x, dim)
+real, intent(in) :: x(:,:)
+integer, optional, value :: dim
+print *, "present(dim), size(a,dim) =", present (dim), size (x,dim=dim)
+if (size (x,dim=dim) /= expect) stop 2
+ end
+end
+
+! Ensure inline code is generated:
+! { dg-final { scan-tree-dump-not "_gfortran_size" "original" } }
--
2.35.3
