Re: [Patch] OpenMP: Fix 'omp declare target' handling for vars [PR99509]
On 12.03.21 13:20, Jakub Jelinek wrote:
On Wed, Mar 10, 2021 at 03:20:42PM +0100, Tobias Burnus wrote:
The C/C++ FE sets for an 'omp declare target' ... 'omp end declare target'
the attribute 'omp declare target implicit'.
That's later processed (for C++) in decl.c - which remove that attribute
and either keeps and explicit 'omp declare target' or 'omp declare target link'
attribute.
Unfortunately, adding 'omp declare target' comes too late as the varpool
has been generated.
Looking attributes on every get rather than just once is IMHO too expensive.
For explicit declare target, we use:
...
(e.g. from cp/parser.c).
So, I think it would be better to do the same thing
I concur. I was already wondering whether it should be in the FE or
not, thinking about some separation between FE and ME. But given
the precedent and the performance issue, it surely makes sense to
solve it in c*/*decl.c.
Done now. OK for mainline?
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
OpenMP: Fix 'omp declare target' handling for vars [PR99509]
For variables with 'declare target' attribute,
varpool_node::get_create marks variables as offload; however,
if the node already exists, it is not updated. C/C++ may tag
decl with 'declare target implicit', which may only be after
varpool creation turned into 'declare target' or 'declare target link';
in this case, the tagging has to happen in the FE.
gcc/c/ChangeLog:
PR c++/99509
* c-decl.c (finish_decl): For 'omp declare target implicit' vars,
ensure that the varpool node is marked as offloadable.
gcc/cp/ChangeLog:
PR c++/99509
* decl.c (cp_finish_decl): For 'omp declare target implicit' vars,
ensure that the varpool node is marked as offloadable.
libgomp/ChangeLog:
PR c++/99509
* testsuite/libgomp.c-c++-common/declare_target-1.c: New test.
gcc/c/c-decl.c | 22 +++---
gcc/cp/decl.c | 21 ++---
.../libgomp.c-c++-common/declare_target-1.c| 22 ++
3 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index b559ed5d76a..3b2241bfd97 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -58,6 +58,9 @@ along with GCC; see the file COPYING3. If not see
#include "c-family/name-hint.h"
#include "c-family/known-headers.h"
#include "c-family/c-spellcheck.h"
+#include "context.h" /* For 'g'. */
+#include "omp-general.h"
+#include "omp-offload.h" /* For offload_vars. */
#include "tree-pretty-print.h"
@@ -5658,9 +5661,22 @@ finish_decl (tree decl, location_t init_loc, tree init,
DECL_ATTRIBUTES (decl))
&& !lookup_attribute ("omp declare target link",
DECL_ATTRIBUTES (decl)))
- DECL_ATTRIBUTES (decl)
- = tree_cons (get_identifier ("omp declare target"),
- NULL_TREE, DECL_ATTRIBUTES (decl));
+ {
+ DECL_ATTRIBUTES (decl)
+ = tree_cons (get_identifier ("omp declare target"),
+ NULL_TREE, DECL_ATTRIBUTES (decl));
+ symtab_node *node = symtab_node::get (decl);
+ if (node != NULL)
+ {
+ node->offloadable = 1;
+ if (ENABLE_OFFLOADING)
+ {
+ g->have_offload = true;
+ if (is_a (node))
+ vec_safe_push (offload_vars, decl);
+ }
+ }
+ }
}
/* This is the last point we can lower alignment so give the target the
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 9c7f6e59bbb..1b671cec9a3 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -53,7 +53,9 @@ along with GCC; see the file COPYING3. If not see
#include "asan.h"
#include "gcc-rich-location.h"
#include "langhooks.h"
+#include "context.h" /* For 'g'. */
#include "omp-general.h"
+#include "omp-offload.h" /* For offload_vars. */
/* Possible cases of bad specifiers type used by bad_specifiers. */
enum bad_spec_place {
@@ -8176,9 +8178,22 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
DECL_ATTRIBUTES (decl))
&& !lookup_attribute ("omp declare target link",
DECL_ATTRIBUTES (decl)))
- DECL_ATTRIBUTES (decl)
- = tree_cons (get_identifier ("omp declare target"),
- NULL_TREE, DECL_ATTRIBUTES (decl));
+ {
+ DECL_ATTRIBUTES (decl)
+ = tree_cons (get_identifier ("omp declare target"),
+ NULL_TREE, DECL_ATTRIBUTES (decl));
+ symtab_node *node = symtab_node::get (decl);
+ if (node != NULL)
+ {
+ node->offloadable = 1;
+ if (ENABLE_OFFLOADING)
+ {
+ g->have_offload = true;
+ if (is_a (node))
+ vec_safe_push (offload_vars, decl);
+ }
+ }
+ }
}
/* This is the last point we can lower alignment so give the target the
diff --git a/libgomp/testsuite/libgomp.c-c++-common/declare_target-1.c b/libgomp/testsuite/libgomp.c-c++-common/declare_target-1.c
new file mode 100644
index 000..c5670dfb7db
--- /dev/null
+++ b/libgomp/tests
Re: [Patch] OpenMP: Fix 'omp declare target' handling for vars [PR99509]
On Mon, Mar 15, 2021 at 09:17:47AM +0100, Tobias Burnus wrote: > gcc/c/ChangeLog: > > PR c++/99509 > * c-decl.c (finish_decl): For 'omp declare target implicit' vars, > ensure that the varpool node is marked as offloadable. > > gcc/cp/ChangeLog: > > PR c++/99509 > * decl.c (cp_finish_decl): For 'omp declare target implicit' vars, > ensure that the varpool node is marked as offloadable. > > libgomp/ChangeLog: > > PR c++/99509 > * testsuite/libgomp.c-c++-common/declare_target-1.c: New test. Ok, thanks. Jakub
Re: [patch, fortran] Fix PR 99345, ICE with DO loop checking
Hello Thomas, hello World, On 14.03.21 21:18, Thomas Koenig via Fortran wrote: the attached, rather obvious patch fixes an ICE on valid which came about because I did not handle EXEC_IOLENGTH as start of an I/O statement when checking for the DO loop variable. This is an 11 regression. Thanks to Harald for reducing this down to the bare minimum. Regression-tested on x86_64-pc-linux-gnu. OK for trunk? OK. Thanks for the patch and thanks to Martin & Harald for the test-case reduction and for Mathias "doko" for the reporting! Tobias Handle EXEC_IOLENGTH in doloop_contained_procedure_code. gcc/fortran/ChangeLog: PR fortran/99345 * frontend-passes.c (doloop_contained_procedure_code): Properly handle EXEC_IOLENGTH. gcc/testsuite/ChangeLog: PR fortran/99345 * gfortran.dg/do_check_16.f90: New test. * gfortran.dg/do_check_17.f90: New test. - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
[Patch, fortran] PR99545 [11 Regression] ICE in gfc_trans_assignment since r11-6253-gce8dcc9105cbd404
Applied to all three branches, after regtesting on each, as blindingly obvious. The testcase is the reduced version in comment #6 of the PR. Paul Fortran: Fix problem with allocate initialization [PR99545]. 2021-03-15 Paul Thomas gcc/fortran/ChangeLog PR fortran/99545 * trans-stmt.c (gfc_trans_allocate): Mark the initialization assignment by setting init_flag. gcc/testsuite/ChangeLog PR fortran/99545 * gfortran.dg/pr99545.f90: New test. diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 547468f7648..7cbdef7a304 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -7001,7 +7001,7 @@ gfc_trans_allocate (gfc_code * code) gfc_expr *init_expr = gfc_expr_to_initialize (expr); gfc_expr *rhs = e3rhs ? e3rhs : gfc_copy_expr (code->expr3); flag_realloc_lhs = 0; - tmp = gfc_trans_assignment (init_expr, rhs, false, false, true, + tmp = gfc_trans_assignment (init_expr, rhs, true, false, true, false); flag_realloc_lhs = realloc_lhs; /* Free the expression allocated for init_expr. */
Re: 12 PR fixed
On Sun, Mar 14, 2021 at 08:22:58AM -0700, Jerry DeLisle wrote:
> > > >
> > > > FAIL: gfortran.dg/pr87907.f90 -O (internal compiler error)
> > > > FAIL: gfortran.dg/pr87907.f90 -O (test for excess errors)
> > > > FAIL: gfortran.dg/pr96013.f90 -O (test for excess errors)
> > > > FAIL: gfortran.dg/pr96025.f90 -O (internal compiler error)
> > > > FAIL: gfortran.dg/pr96025.f90 -O (test for errors, line 5)
> > > > FAIL: gfortran.dg/pr96025.f90 -O (test for excess errors)
> > >
No idea why I don't see the above. This patch on top of
the previous patch might fix the last 3 FAILs. (Watch
for copy-n-paste whitespace corruption.)
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index e1acc2db000..081487a45e6 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -3476,7 +3476,6 @@ gfc_specification_expr (gfc_expr *e)
{
gfc_error ("Expression at %L must be of INTEGER type, found %s",
&e->where, gfc_basic_typename (e->ts.type));
- gfc_clear_ts (&e->ts);
return false;
}
@@ -5246,11 +5245,12 @@ gfc_traverse_expr (gfc_expr *expr, gfc_symbol *sym,
if ((*func) (expr, sym, &f))
return true;
- if (expr->ts.type == BT_CHARACTER
- && expr->ts.u.cl
- && expr->ts.u.cl->length
- && expr->ts.u.cl->length->expr_type != EXPR_CONSTANT
- && gfc_traverse_expr (expr->ts.u.cl->length, sym, func, f))
+ if (expr->expr_type != EXPR_CONSTANT
+ && expr->ts.type == BT_CHARACTER
+ && expr->ts.u.cl
+ && expr->ts.u.cl->length
+ && expr->ts.u.cl->length->expr_type != EXPR_CONSTANT
+ && gfc_traverse_expr (expr->ts.u.cl->length, sym, func, f))
return true;
switch (expr->expr_type)
--
steve
