Re: [patch] OpenMP: Add -Wopenmp and use it
Hi! On Fri, 24 Nov 2023 at 15:08, Jakub Jelinek wrote: > > On Fri, Nov 24, 2023 at 02:51:28PM +0100, Tobias Burnus wrote: > > Following the general trend to add a "[-W...]" to the warning messages > > for both better grouping of the warnings and - more importantly - for > > providing > > a means to silence such a warning (or to -Werror= them explicitly), this > > patch > > replaces several '0' by OPT_Wopenmp. > > > > Comments or remarks before I commit it? > > LGTM, thanks for working on it. > > Jakub > I think the lack of final '.' in: gcc/c-family/c.opt + Warn about suspicious OpenMP code has caused the following regressions: Running gcc:gcc.misc-tests/help.exp ... FAIL: compiler driver --help=c option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" FAIL: compiler driver --help=c++ option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" FAIL: compiler driver --help=warnings option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" I think you have received a notification from our CI about that? Can you check it's as simple as that? Thanks, Christophe
Re: [patch] OpenMP: Add -Wopenmp and use it
On Mon, Nov 27, 2023 at 11:20:20AM +0100, Christophe Lyon wrote: > On Fri, 24 Nov 2023 at 15:08, Jakub Jelinek wrote: > > > Comments or remarks before I commit it? > > > > LGTM, thanks for working on it. > > > > Jakub > > > > I think the lack of final '.' in: > gcc/c-family/c.opt > + Warn about suspicious OpenMP code Tobias has fixed that a few commits later: r14-5835-g6eb1507107dee3e67e3a136e2917b93cdffba7c4 Sorry for missing that during patch review. Jakub
Re: [patch] OpenMP: Add -Wopenmp and use it
Hi, On 27.11.23 11:20, Christophe Lyon wrote: I think the lack of final '.' in: Indeed - but you are lagging a bit behind: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638128.html [committed] c-family/c.opt (-Wopenmp): Add missing tailing '.' Fri Nov 24 18:56:21 GMT 2023 Committed as r14-5835-g6eb1507107dee3 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] OpenMP: Add -Wopenmp and use it
On Mon, 27 Nov 2023 at 11:33, Tobias Burnus wrote: > > Hi, > > On 27.11.23 11:20, Christophe Lyon wrote: > > > I think the lack of final '.' in: > > Indeed - but you are lagging a bit behind: > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638128.html > > [committed] c-family/c.opt (-Wopenmp): Add missing tailing '.' > > Fri Nov 24 18:56:21 GMT 2023 > > Committed as r14-5835-g6eb1507107dee3 > Great thanks! Sorry for the noise, it's a bit hard and error-prone to track which regressions have already fixed and/or are being worked on. Our bisect started at r14-5830, just a bit too early :-) Thanks, Christophe > 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 v2] Fortran: fix reallocation on assignment of polymorphic variables [PR110415]
This is the second version of the patch - previous discussion at:
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636671.html
This patch adds the testcase from PR110415 and fixes the bug.
The problem is that in a couple of places in trans_class_assignment in
trans-expr.cc, we need to get the run-time size of the polymorphic
object from the vtbl, but we are currently getting that vtbl from the
lhs of the assignment rather than the rhs. This gives us the old value
of the size but we need to pass the new size to __builtin_malloc and
__builtin_realloc.
I'm fixing this by adding a parameter to trans_class_vptr_len_assignment
to retrieve the tree corresponding the vptr from the object on the rhs
of the assignment, and then passing this where it is needed. In the case
where trans_class_vptr_len_assignment returns NULL_TREE for the rhs vptr
we use the lhs vptr as before.
To get this to work I also needed to change the implementation of
trans_class_vptr_len_assignment to create a temporary for the assignment
in more circumstances. Currently, the "a = func()" assignment in MAIN__
doesn't hit the "Create a temporary for complication expressions" case
on line 9951 because "DECL_P (rse->expr)" is true - the expression has
already been placed into a temporary. That means we don't hit the "if
(temp_rhs ..." case on line 10038 and go on to get the vptr_expr from
"gfc_lval_expr_from_sym (gfc_find_vtab (&re->ts))" on line 10057 which
is the vtbl of the static type rather than the dynamic one from the rhs.
So with this fix we create an extra temporary, but that should be
optimised away in the middle-end so there should be no run-time effect.
I'm not sure if this is the best way to fix this (the Fortran front-end
is new territory for me) but I've verified that the testcase passes with
this change, fails without it, and that the change does not introduce
any FAILs when running the gfortran testcases on x86_64-pc-linux-gnu.
After the previous submission, Tobias Burnus found a closely related
problem and contributed testcases and a fix for it, which I have
incorporated into this version of the patch. The problem in this case is
with the __builtin_realloc call that is executed if one polymorphic
variable is replaced by another. The return value of this call was being
ignored rather than used to replace the pointer being reallocated.
Is this OK for mainline, GCC 13 and OG13?
Thanks,
Andrew
gcc/fortran/
PR fortran/110415
* trans-expr.cc (trans_class_vptr_len_assignment): Add
from_vptrp parameter. Populate it. Don't check for DECL_P
when deciding whether to create temporary.
(trans_class_pointer_fcn, gfc_trans_pointer_assignment): Add
NULL argument to trans_class_vptr_len_assignment calls.
(trans_class_assignment): Get rhs_vptr from
trans_class_vptr_len_assignment and use it for determining size
for allocation/reallocation. Use return value from realloc.
gcc/testsuite/
PR fortran/110415
* gfortran.dg/pr110415.f90: New test.
* gfortran.dg/asan/pr110415-2.f90: New test.
* gfortran.dg/asan/pr110415-3.f90: New test.diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 50c4604a025..bfe9996ced6 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -9936,7 +9936,8 @@ trans_get_upoly_len (stmtblock_t *block, gfc_expr *expr)
static tree
trans_class_vptr_len_assignment (stmtblock_t *block, gfc_expr * le,
gfc_expr * re, gfc_se *rse,
-tree * to_lenp, tree * from_lenp)
+tree * to_lenp, tree * from_lenp,
+tree * from_vptrp)
{
gfc_se se;
gfc_expr * vptr_expr;
@@ -9944,10 +9945,11 @@ trans_class_vptr_len_assignment (stmtblock_t *block,
gfc_expr * le,
bool set_vptr = false, temp_rhs = false;
stmtblock_t *pre = block;
tree class_expr = NULL_TREE;
+ tree from_vptr = NULL_TREE;
/* Create a temporary for complicated expressions. */
if (re->expr_type != EXPR_VARIABLE && re->expr_type != EXPR_NULL
- && rse->expr != NULL_TREE && !DECL_P (rse->expr))
+ && rse->expr != NULL_TREE)
{
if (re->ts.type == BT_CLASS && !GFC_CLASS_TYPE_P (TREE_TYPE (rse->expr)))
class_expr = gfc_get_class_from_expr (rse->expr);
@@ -10044,6 +10046,7 @@ trans_class_vptr_len_assignment (stmtblock_t *block,
gfc_expr * le,
tmp = rse->expr;
se.expr = gfc_class_vptr_get (tmp);
+ from_vptr = se.expr;
if (UNLIMITED_POLY (re))
from_len = gfc_class_len_get (tmp);
@@ -10065,6 +10068,7 @@ trans_class_vptr_len_assignment (stmtblock_t *block,
gfc_expr * le,
gfc_free_expr (vptr_expr);
gfc_add_block_to_block (block, &se.pre);
gcc_assert (se.post.head == NULL_TREE);
+ from_vptr = se.expr;
}
gfc_add_modify (pr
[PATCH] Fortran: deferred-length character optional dummy arguments [PR93762,PR100651]
Dear all,
the attached patch fixes the passing of deferred-length character
to optional dummy arguments: the character length shall be passed
by reference, not by value.
Original analysis of the issue by Steve in PR93762, independently
done by FX in PR100651. The patch fixes both PRs.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
As the fix is local and affects only deferred-length character,
would it be ok to backport to 13-branch?
Thanks,
Harald
From 8ce1c8e7d0390361a1507000b7abbf6509b2fee9 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Mon, 27 Nov 2023 20:19:11 +0100
Subject: [PATCH] Fortran: deferred-length character optional dummy arguments
[PR93762,PR100651]
gcc/fortran/ChangeLog:
PR fortran/93762
PR fortran/100651
* trans-expr.cc (gfc_conv_missing_dummy): The character length for
deferred-length dummy arguments is passed by reference, so that its
value can be returned. Adjust handling for optional dummies.
gcc/testsuite/ChangeLog:
PR fortran/93762
PR fortran/100651
* gfortran.dg/optional_deferred_char_1.f90: New test.
---
gcc/fortran/trans-expr.cc | 22 +++-
.../gfortran.dg/optional_deferred_char_1.f90 | 100 ++
2 files changed, 118 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/optional_deferred_char_1.f90
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 50c4604a025..e992f60d8bb 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -2116,10 +2116,24 @@ gfc_conv_missing_dummy (gfc_se * se, gfc_expr * arg, gfc_typespec ts, int kind)
if (ts.type == BT_CHARACTER)
{
- tmp = build_int_cst (gfc_charlen_type_node, 0);
- tmp = fold_build3_loc (input_location, COND_EXPR, gfc_charlen_type_node,
- present, se->string_length, tmp);
- tmp = gfc_evaluate_now (tmp, &se->pre);
+ /* Handle deferred-length dummies that pass the character length by
+ reference so that the value can be returned. */
+ if (ts.deferred && INDIRECT_REF_P (se->string_length))
+ {
+ tmp = gfc_build_addr_expr (NULL_TREE, se->string_length);
+ tmp = fold_build3_loc (input_location, COND_EXPR, TREE_TYPE (tmp),
+ present, tmp, null_pointer_node);
+ tmp = gfc_evaluate_now (tmp, &se->pre);
+ tmp = build_fold_indirect_ref_loc (input_location, tmp);
+ }
+ else
+ {
+ tmp = build_int_cst (gfc_charlen_type_node, 0);
+ tmp = fold_build3_loc (input_location, COND_EXPR,
+ gfc_charlen_type_node,
+ present, se->string_length, tmp);
+ tmp = gfc_evaluate_now (tmp, &se->pre);
+ }
se->string_length = tmp;
}
return;
diff --git a/gcc/testsuite/gfortran.dg/optional_deferred_char_1.f90 b/gcc/testsuite/gfortran.dg/optional_deferred_char_1.f90
new file mode 100644
index 000..d399dd11ca2
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/optional_deferred_char_1.f90
@@ -0,0 +1,100 @@
+! { dg-do run }
+! PR fortran/93762
+! PR fortran/100651 - deferred-length character as optional dummy argument
+
+program main
+ implicit none
+ character(:), allocatable :: err_msg, msg3(:)
+ character(:), pointer :: err_msg2 => NULL()
+
+ ! Subroutines with optional arguments
+ call to_int ()
+ call to_int_p ()
+ call test_rank1 ()
+ call assert_code ()
+ call assert_p ()
+ call assert_rank1 ()
+
+ ! Test passing of optional arguments
+ call to_int (err_msg)
+ if (.not. allocated (err_msg)) stop 1
+ if (len (err_msg) /= 7)stop 2
+ if (err_msg(1:7) /= "foo bar") stop 3
+
+ call to_int2 (err_msg)
+ if (.not. allocated (err_msg)) stop 4
+ if (len (err_msg) /= 7)stop 5
+ if (err_msg(1:7) /= "foo bar") stop 6
+ deallocate (err_msg)
+
+ call to_int_p (err_msg2)
+ if (.not. associated (err_msg2)) stop 11
+ if (len (err_msg2) /= 8) stop 12
+ if (err_msg2(1:8) /= "poo bla ") stop 13
+ deallocate (err_msg2)
+
+ call to_int2_p (err_msg2)
+ if (.not. associated (err_msg2)) stop 14
+ if (len (err_msg2) /= 8) stop 15
+ if (err_msg2(1:8) /= "poo bla ") stop 16
+ deallocate (err_msg2)
+
+ call test_rank1 (msg3)
+ if (.not. allocated (msg3)) stop 21
+ if (len (msg3) /= 2)stop 22
+ if (size (msg3) /= 42) stop 23
+ if (any (msg3 /= "ok")) stop 24
+ deallocate (msg3)
+
+contains
+
+ ! Deferred-length character, allocatable:
+ subroutine assert_code (err_msg0)
+character(:), optional, allocatable :: err_msg0
+if (present (err_msg0)) err_msg0 = 'foo bar'
+ end
+ ! Test: optional argument
+ subroutine to_int (err_msg1)
+character(:), optional, allocatable :: err_msg1
+call assert_code (err_msg1)
+ end
+ ! Control: non-optional argument
+ subroutine to_int2 (err_msg2)
+character(:), allocatable :: err_msg2
+call assert_code (err_msg2)
+ end
+
+ ! Rank-1:
+ subroutine assert_rank1 (msg)
+character(:), optional, allocatable, intent(out) :: msg(:)
+if (present (msg)) then
+ allocate (character(2) :: msg(42))
+ msg(:) = "ok"
+end if
+
