Re: [Patch, Fortran, Update 2] PR98301 Re: RANDOM_INIT() and coarray Fortran
Hi Steve, hi all,
I agree. The cas-things have been removed (I will put the patch for them into
the pr98301 ticket, so safe them), streamlining the patch a bit more.
Bootstraped and regtested ok on x86_64-linux/f33. Ok for trunk?
Regards,
Andre
Steve Kargl
PR fortran/98301 - random_init() is broken
Correct implementation of random_init() when -fcoarray=lib is given.
gcc/fortran/ChangeLog:
PR fortran/98301
* trans-decl.c (gfc_build_builtin_function_decls): Move decl.
* trans-intrinsic.c (conv_intrinsic_random_init): Use bool for
lib-call of caf_random_init instead of logical (4-byte).
* trans.h: Add tree var for random_init.
libgfortran/ChangeLog:
PR fortran/98302
* caf/libcaf.h (_gfortran_caf_random_init): New function.
* caf/single.c (_gfortran_caf_random_init): New function.
* gfortran.map: Added fndecl.
* intrinsics/random_init.f90: Implement random_init.
On Sun, 25 Apr 2021 13:03:34 -0700
Steve Kargl wrote:
> Andre,
>
> The patch looks fine to me. I wonder, however, if we should
> comment out all of the shared memory stuff, i.e., the _cas_
> stuff. I don't know when Thomas/Nicolas will merge their
> work-in-progress.
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index cc9d85543ca..7365dde47bf 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -170,6 +170,7 @@ tree gfor_fndecl_co_min;
tree gfor_fndecl_co_reduce;
tree gfor_fndecl_co_sum;
tree gfor_fndecl_caf_is_present;
+tree gfor_fndecl_caf_random_init;
/* Math functions. Many other math functions are handled in
@@ -233,7 +234,7 @@ tree gfor_fndecl_cgemm;
tree gfor_fndecl_zgemm;
/* RANDOM_INIT function. */
-tree gfor_fndecl_random_init;
+tree gfor_fndecl_random_init; /* libgfortran, 1 image only. */
static void
gfc_add_decl_to_parent_function (tree decl)
@@ -3515,6 +3516,8 @@ gfc_build_intrinsic_function_decls (void)
void_type_node, 3, gfc_logical4_type_node, gfc_logical4_type_node,
gfc_int4_type_node);
+ // gfor_fndecl_caf_rand_init is defined in the lib-coarray section below.
+
gfor_fndecl_sc_kind = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX("selected_char_kind")), ". . R ",
gfc_int4_type_node, 2, gfc_charlen_type_node, pchar_type_node);
@@ -4080,6 +4083,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX("caf_is_present")), ". r . r ",
integer_type_node, 3, pvoid_type_node, integer_type_node,
pvoid_type_node);
+
+ gfor_fndecl_caf_random_init = gfc_build_library_function_decl (
+ get_identifier (PREFIX("caf_random_init")),
+ void_type_node, 2, logical_type_node, logical_type_node);
}
gfc_build_intrinsic_function_decls ();
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index cceef8f34ac..3f38ec8de85 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -3827,38 +3827,43 @@ conv_intrinsic_random_init (gfc_code *code)
{
stmtblock_t block;
gfc_se se;
- tree arg1, arg2, arg3, tmp;
- tree logical4_type_node = gfc_get_logical_type (4);
+ tree arg1, arg2, tmp;
+ /* On none coarray == lib compiles use LOGICAL(4) else regular LOGICAL. */
+ tree used_bool_type_node = flag_coarray == GFC_FCOARRAY_LIB
+ ? logical_type_node
+ : gfc_get_logical_type (4);
/* Make the function call. */
gfc_init_block (&block);
gfc_init_se (&se, NULL);
- /* Convert REPEATABLE to a LOGICAL(4) entity. */
+ /* Convert REPEATABLE to the desired LOGICAL entity. */
gfc_conv_expr (&se, code->ext.actual->expr);
gfc_add_block_to_block (&block, &se.pre);
- arg1 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+ arg1 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
gfc_add_block_to_block (&block, &se.post);
- /* Convert IMAGE_DISTINCT to a LOGICAL(4) entity. */
+ /* Convert IMAGE_DISTINCT to the desired LOGICAL entity. */
gfc_conv_expr (&se, code->ext.actual->next->expr);
gfc_add_block_to_block (&block, &se.pre);
- arg2 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+ arg2 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
gfc_add_block_to_block (&block, &se.post);
- /* Create the hidden argument. For non-coarray codes and -fcoarray=single,
- simply set this to 0. For -fcoarray=lib, generate a call to
- THIS_IMAGE() without arguments. */
- arg3 = build_int_cst (gfc_get_int_type (4), 0);
if (flag_coarray == GFC_FCOARRAY_LIB)
{
- arg3 = build_call_expr_loc (input_location, gfor_fndecl_caf_this_image,
- 1, arg3);
- se.expr = fold_convert (gfc_get_int_type (4), arg3);
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_random_init,
+ 2, arg1, arg2);
+}
+ else
+{
+ /* The ABI for libgfortran needs to be maintained, s
[Patch, fortran v2] PR fortran/92621 Problems with memory handling with allocatable intent(out) arrays with bind(c)
Hi all! Proposed patch to: PR92621 - Problems with memory handling with allocatable intent(out) arrays with bind(c) Patch tested only on x86_64-pc-linux-gnu. The code currently generated tries to deallocate the undefined artificial cfi.n pointer before it is associated with the allocatable array. Since the cfi.n pointer is undefined attempting to free it is really a bad idea and it will frequently segfault. Consequently, since the deallocation is done before the cfi.n pointer is associated with anything, the allocatable array is never freed, like it should, and it will be passed still allocated causing subsequent attempts to allocate it to fail. Version 2 is basically a ping, fixes a typo, replaces an if block with a flag to make reviewing easier and replaces a call to malloc with calloc to make Valgrind happy. Thank you very much. Best regards, José Rui Fortran: Fix segfaults due to freeing undefined pointer [PR92621] gcc/fortran/ChangeLog: PR fortran/92621 * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Add code to deallocate allocatable intent(out) dummy array arguments and slightly rearrange code. (gfc_conv_procedure_call): Add a flag to avoid double frees, removes unnecessary checks for bind(c) objects and obsolete comments. libgfortran/ChangeLog: PR fortran/92621 * runtime/ISO_Fortran_binding.c (gfc_desc_to_cfi_desc): replaces a call to malloc with calloc to make Valgrind happy. gcc/testsuite/ChangeLog: PR fortran/92621 * gfortran.dg/bind-c-intent-out.f90: Changes regex to match the changes in code generation. * gfortran.dg/PR92621.f90: Improved new test.
[wwwdocs, patch] gcc-12/changes.html: OpenMP (depobj/mutexinoutset for depend), OpenACC (-Wopenacc-parallelism)
Hi all, this patch documents a new OpenMP feature (many more to be added) in GCC 12. And it documents a new flag for OpenACC. Comments? Wording suggestions? I think for OpenMP, the sentence will be modified several times before the release :-) Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf diff --git a/htdocs/gcc-12/changes.html b/htdocs/gcc-12/changes.html index 8f257e87..1022b5eb 100644 --- a/htdocs/gcc-12/changes.html +++ b/htdocs/gcc-12/changes.html @@ -40,6 +40,18 @@ a work-in-progress. New Languages and Language specific improvements + + For Fortran, OpenMP 5.0 support has been extended for following features + which were before only available in C and C++: depobj + and mutexinoutset can now also be used with the + depend clause. + + The new warning flag -Wopenacc-parallelism was added for + OpenACC, which warns about potentially suboptimal choices related to + OpenACC parallelism. + + +
Re: [wwwdocs, patch] gcc-12/changes.html: OpenMP (depobj/mutexinoutset for depend), OpenACC (-Wopenacc-parallelism)
On Mon, 26 Apr 2021, Tobias Burnus wrote: > Comments? Wording suggestions? I think for OpenMP, the sentence will be > modified several times before the release :-) Can I take this as a promise? :-) + + For Fortran, OpenMP 5.0 support has been extended for following features + which were before only available in C and C++: depobj + and mutexinoutset can now also be used with the + depend clause. + I've been staring at this multiple times thinking on how to make it a bit stronger/clearer. How do you feel about "OpenMP 5.0 support for Fortran ... by the following features which were available in C and C++ before:" Or "Fortran gained support for the following OpenMP 5.0 features..." ? (In marketing, which this kinds is, I suggest not saying "only" since that risks diminishing the value we've had all along. Rather it's "even more/better now". ;-) + The new warning flag -Wopenacc-parallelism was added for + OpenACC, which warns about potentially suboptimal choices related to + OpenACC parallelism. + Maybe break this as "... It warns about"? That feels a bit more natural to me than the "which" referring to an earlier term, though that may very well be my subjective preference. ;-) Please consider the above, and then it's okay, however you want to go about it. Gerald
Re: [PATCH] openacc: Warnings for strange partitioning choices for parallel regions
Hi!
On 2021-04-26T14:32:10+0200, Tobias Burnus wrote:
> first, can you add an entry regarding this flag
> to https://gcc.gnu.org/gcc-12/changes.html ?
Is that a standard thing that all new command-line flags do get
highlighted there? (I wouldn't have considered that one important
enough?)
> Secondly, I now see FAILs like:
>
> FAIL: gfortran.dg/goacc/classify-serial.f95 -O (test for excess errors)
> Excess errors:
> gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains gang
> partitioned code but is not gang partitioned [-Wopenacc-parallelism]
> gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains vector
> partitioned code but is not vector partitioned [-Wopenacc-parallelism]
Eek! I do reproduce that -- but only in a GCC build configuration
without offloading enabled! For example, for
'gfortran.dg/goacc/classify-serial.f95', we've got the following diff in
diagnostics in a GCC build configuration without vs. with offloading
enabled:
{+[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95: In function
'MAIN__._omp_fn.0':+}
[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132:
[-Warning:-]{+warning:+} region contains gang partitioned code but is not gang
partitioned [-Wopenacc-parallelism]
[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132:
[-Warning:-]{+warning:+} region contains vector partitioned code but is not
vector partitioned [-Wopenacc-parallelism]
[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132:
optimized: assigned OpenACC gang vector loop parallelism
PASS: gfortran.dg/goacc/classify-serial.f95 -O (test for warnings, line
20)
[-XPASS:-]{+XFAIL:+} gfortran.dg/goacc/classify-serial.f95 -O TODO
'serial' (test for bogus messages, line 20)
PASS: gfortran.dg/goacc/classify-serial.f95 -O (test for bogus
messages, line 20)
[-XPASS:-]{+XFAIL:+} gfortran.dg/goacc/classify-serial.f95 -O TODO
'serial' (test for bogus messages, line 20)
[-FAIL:-]{+PASS:+} gfortran.dg/goacc/classify-serial.f95 -O (test for
excess errors)[-Excess errors:-]
[-[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132:
Warning: region contains gang partitioned code but is not gang partitioned
[-Wopenacc-parallelism]-]
[-[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132:
Warning: region contains vector partitioned code but is not vector partitioned
[-Wopenacc-parallelism]-]
PASS: gfortran.dg/goacc/classify-serial.f95 -O scan-tree-dump-times
ompexp "(?n)__attribute__\\(\\(oacc serial, omp target entrypoint\\)\\)" 1
PASS: gfortran.dg/goacc/classify-serial.f95 -O scan-tree-dump-times
oaccdevlow "(?n)Function is OpenACC serial offload" 1
PASS: gfortran.dg/goacc/classify-serial.f95 -O scan-tree-dump-times
oaccdevlow "(?n)Compute dimensions \\[1, 1, 1\\]" 1
Notice upper-case "Warning" vs. lower-case "warning". That's for Fortran
only; for C, C++, it's lower-case "warning" for both build variants.
It's of course easy to fix up the regexp, but should we maybe rather
figure out how to unify the reporting?
I do understand that Fortran has some upper-case diagnostics, but I don't
understand the difference/relevance of GCC build configuration without
vs. with offloading enabled, how is that coming becoming relevant?
Also notice that the preamble "In function 'MAIN__._omp_fn.0':" only
appears in the GCC build configuration with offloading. Looking at
'c-c++-common/goacc/classify-serial.c', for C, we've got "In function
'SERIAL._omp_fn.0':" for both GCC build configurations, and for C++,
we've got "In function '_Z6SERIALv._omp_fn.0':" without offloading
enabled vs. "In function 'SERIAL() [clone ._omp_fn.0]':" with offloading
enabled.
So there generally seems to be some difference in function outlining
setup without vs. with offloading enabled? ..., and for Fortran, with
offloading enabled, that causes the outlined function to be
"de-Fortranified", thus the lower-case "warning" diagnostic?
> FAIL: gfortran.dg/goacc/kernels-decompose-2.f95 -O (test for excess errors)
> Excess errors:
> gfortran.dg/goacc/kernels-decompose-2.f95:124:15: Warning: region contains
> gang partitioned code but is not gang partitioned [-Wopenacc-parallelism]
>
> FAIL: gfortran.dg/goacc/routine-module-mod-1.f90 (test for excess errors)
> Excess errors:
> gfortran.dg/goacc/routine-module-mod-1.f90:56:16: Warning: region is worker
> partitioned but does not contain worker partitioned code
> [-Wopenacc-parallelism]
..., and similarly, several more in 'libgomp.oacc-fortran'.
Grüße
Thomas
> On 26.04.21 12:39, Thomas Schwinge wrote:
>
>> Hi!
>>
>> On 2021-02-26T04:21:54-0800, Julian Brown wrote:
>>> This patch adds warnings for strange partitioning choices -- specifying
>>> either more or fewer partitioning levels on a parallel directive than the
>>> enclosed offload region actually uses.
>> Thanks!
>>
>>> We've used a version of this patch o
Re: [PATCH] openacc: Warnings for strange partitioning choices for parallel regions
On 26.04.21 21:54, Thomas Schwinge wrote:
On 2021-04-26T14:32:10+0200, Tobias Burnus wrote:
first, can you add an entry regarding this flag
to https://gcc.gnu.org/gcc-12/changes.html ?
Is that a standard thing that all new command-line flags do get
highlighted there? (I wouldn't have considered that one important
enough?)
I think options which one expects to to use as user – especially when
they are for consideration with -W... do make sense. In general, as a
user I am happy to have a more detailed information about changes than
'something has changed'.
Secondly, I now see FAILs like:
FAIL: gfortran.dg/goacc/classify-serial.f95 -O (test for excess errors)
Excess errors:
gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains gang
partitioned code but is not gang partitioned [-Wopenacc-parallelism]
gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains vector
partitioned code but is not vector partitioned [-Wopenacc-parallelism]
Eek! I do reproduce that -- but only in a GCC build configuration
without offloading enabled!
Aha, that explains why you did commit it like that – now I have fixed
the nonoffload version – and broken the offloaded one.
Notice upper-case "Warning" vs. lower-case "warning". That's for Fortran
only; for C, C++, it's lower-case "warning" for both build variants.
It's of course easy to fix up the regexp, but should we maybe rather
figure out how to unify the reporting?
I do understand that Fortran has some upper-case diagnostics, but I don't
understand the difference/relevance of GCC build configuration without
vs. with offloading enabled, how is that coming becoming relevant?
The default is 'global_dc = &global_diagnostic_context' which gets
overridden in fortran/error.c.
I have no idea why there is a difference but wonder whether it has
something to do with taking the LTO path (flag_generate_offload) and
some overriding of the settings.
I believe this is done in tree.c's free_lang_data:
if (in_lto_p
|| (!flag_generate_lto && !flag_generate_offload))
...
return 0;
}
/* Reset diagnostic machinery. */
tree_diagnostics_defaults (global_dc);
And in cgraphunit.c's symbol_table::finalize_compilation_unit
if (!in_lto_p && g->have_offload)
flag_generate_offload = 1;
And g->have_offload is guarded by ENABLE_OFFLOAD.
Also notice that the preamble "In function 'MAIN__._omp_fn.0':" only
appears in the GCC build configuration with offloading. Looking at
'c-c++-common/goacc/classify-serial.c', for C, we've got "In function
'SERIAL._omp_fn.0':" for both GCC build configurations, and for C++,
we've got "In function '_Z6SERIALv._omp_fn.0':" without offloading
enabled vs. "In function 'SERIAL() [clone ._omp_fn.0]':" with offloading
enabled.
I note that:
langhooks.c: (context->printer, _("In function %qs"),
and in cp/error.c's cp_print_error_function:
pp_printf (context->printer, function_category (fndecl),
cxx_printable_name_translate (fndecl, 2));
My bet is that here the same happens as above – the global_dc gets
reset – and the diagnostic becomes different.
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
[Patch, committed] OpenACC: Fix pattern in dg-bogus in Fortran testcases again (Re: [PATCH] openacc: Warnings for strange partitioning choices for parallel regions)
Hi all,
as discussed below/in this thread, the reset of the diagnostic
contest affects the output with ENABLE_OFFLOAD. The attached
patch now uses [Ww]arning in dg-bogus.
It might be possible to reduce the differences between
ENABLE_OFFLOAD being true or false, but it does not seem to
be simple and may not be worth the effort.
In any case, this patch should permit to have no FAIL
with ENABLE_OFFLOAD set and unset. – And should make the
testers happy.
Committed as r12-135-gbd7ebe9da745a62184052dd1b15f4dd10fbdc9f4
Tobias
PS: 'Warning:' with Fortran diagnostic and ENABLE_OFFSET unset,
'warning:' with default diagnostic and ENABLE_OFFSET set – due to:
On 26.04.21 22:51, Tobias Burnus wrote:
I believe this is done in tree.c's free_lang_data:
if (in_lto_p
|| (!flag_generate_lto && !flag_generate_offload))
...
return 0;
}
/* Reset diagnostic machinery. */
tree_diagnostics_defaults (global_dc);
And in cgraphunit.c's symbol_table::finalize_compilation_unit
if (!in_lto_p && g->have_offload)
flag_generate_offload = 1;
And g->have_offload is guarded by ENABLE_OFFLOAD.
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
commit bd7ebe9da745a62184052dd1b15f4dd10fbdc9f4
Author: Tobias Burnus
Date: Mon Apr 26 22:59:21 2021 +0200
OpenACC: Fix pattern in dg-bogus in Fortran testcases again
It turned out that a compiler built without offloading support
and one with can produce slightly different diagnostic.
Offloading support implies ENABLE_OFFLOAD which implies that
g->have_offload is set when offloading is actually needed.
In cgraphunit.c, the latter causes flag_generate_offload = 1,
which in turn affects tree.c's free_lang_data.
The result is that the front-end specific diagnostic gets reset
('tree_diagnostics_defaults (global_dc)'), which affects in this
case 'Warning' vs. 'warning' via the Fortran frontend.
Result: 'Warning:' vs. 'warning:'.
Side note: Other FE also override the diagnostic, leading to
similar differences, e.g. the C++ FE outputs mangled function
names differently, cf. patch thread.
libgomp/ChangeLog:
* testsuite/libgomp.oacc-fortran/par-reduction-2-1.f:
Use [Ww]arning in dg-bogus as FE diagnostic and default
diagnostic differ and the result depends on ENABLE_OFFLOAD.
* testsuite/libgomp.oacc-fortran/par-reduction-2-2.f: Likewise.
* testsuite/libgomp.oacc-fortran/parallel-dims.f90: Likewise.
* testsuite/libgomp.oacc-fortran/parallel-reduction.f90: Likewise.
gcc/testsuite/ChangeLog:
* gfortran.dg/goacc/classify-serial.f95:
Use [Ww]arning in dg-bogus as FE diagnostic and default
diagnostic differ and the result depends on ENABLE_OFFLOAD.
* gfortran.dg/goacc/kernels-decompose-2.f95: Likewise.
* gfortran.dg/goacc/routine-module-mod-1.f90: Likewise.
---
gcc/testsuite/gfortran.dg/goacc/classify-serial.f95 | 6 +++---
gcc/testsuite/gfortran.dg/goacc/kernels-decompose-2.f95 | 2 +-
gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90 | 2 +-
libgomp/testsuite/libgomp.oacc-fortran/par-reduction-2-1.f| 4 ++--
libgomp/testsuite/libgomp.oacc-fortran/par-reduction-2-2.f| 4 ++--
libgomp/testsuite/libgomp.oacc-fortran/parallel-dims.f90 | 6 +++---
libgomp/testsuite/libgomp.oacc-fortran/parallel-reduction.f90 | 4 ++--
7 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95 b/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95
index 386b95e78e4..6dcb1b170f8 100644
--- a/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95
@@ -18,9 +18,9 @@ program main
call setup(a, b)
!$acc serial loop copyin (a(0:n-1), b(0:n-1)) copyout (c(0:n-1)) ! { dg-message "optimized: assigned OpenACC gang vector loop parallelism" }
- ! { dg-bogus "Warning: region contains gang partitioned code but is not gang partitioned" "TODO 'serial'" { xfail *-*-* } .-1 }
- ! { dg-bogus "Warning: region contains worker partitioned code but is not worker partitioned" "" { target *-*-* } .-2 }
- ! { dg-bogus "Warning: region contains vector partitioned code but is not vector partitioned" "TODO 'serial'" { xfail *-*-* } .-3 }
+ ! { dg-bogus "\[Ww\]arning: region contains gang partitioned code but is not gang partitioned" "TODO 'serial'" { xfail *-*-* } .-1 }
+ ! { dg-bogus "\[Ww\]arning: region contains worker partitioned code but is not worker partitioned" "" { target *-*-* } .-2 }
+ ! { dg-bogus "\[Ww\]arning: region contains vector partitioned code but is not vector partitioned" "TODO 'serial'" { xfail *-*-* } .-3 }
do i = 0, n - 1
c(i) = a(i) + b(i)
end do
diff --git a/gcc/testsuite/gfortr
