Re: [PATCH 00/14] fortran: Use precalculated class container for deallocation [PR110618]
Le 15/07/2023 à 08:11, Paul Richard Thomas a écrit : Please either remove or uncomment the line: // gcc_assert (se.pre.head == NULL_TREE && se.post.head == NULL_TREE); I presume that it reflects some case where the assertion failed? If so, it might be prudent to retain the assertion especially in light of: gcc_assert (tmp_se.post.head == NULL_TREE); a bit further down. I have finally found an example making the assert fail and pushed the patches with the assert removed and the test added to the testsuite.
Re: PR82943 - Suggested patch to fix
Hello, I wanted to follow up on this, and ask what the next steps would be to incorporate this patch? Thanks, Alexander Westbrooks On Thu, Jun 29, 2023 at 10:38 PM Alexander Westbrooks wrote: > Hello, > > I have finished my testing, and updated my patch and relevant Changelogs. > I added 4 new tests and all the existing tests in the current testsuite > for gfortran passed or failed as expected. Do I need to attach the test > results here? > > The platform I tested on was a Docker container running in Docker Desktop, > running the "mcr.microsoft.com/devcontainers/universal:2-linux" image. > > I also made sure that my code changes followed the coding standards. > Please let me know if there is anything else that I need to do. I don't > have write-access to the repository. > > Thanks, > > Alexander > > On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf wrote: > >> Hi Alex, >> >> welcome to the gfortran community. It is great that you are trying >> to get actively involved. >> >> You already did quite a few things right: patches shall be sent to >> the gcc-patches ML, but Fortran reviewers usually notice them only >> where they are copied to the fortran ML. >> >> There are some general recommendations on the formatting of C code, >> like indentation, of the patches, and of the commit log entries. >> >> Regarding coding standards, see https://www.gnu.org/prep/standards/ . >> >> Regarding testcases, a recommendation is to have a look at >> existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then >> decide if the testcase shall test the compile-time or run-time >> behaviour, and add the necessary dejagnu directives. >> >> You should also verify if your patch passes regression testing. >> For changes to gfortran, it is usually sufficient to run >> >> make check-fortran -j >> >> where is the number of parallel tests. >> You would need to report also the platform where you tested on. >> >> There is also a legal issue to consider before non-trivial patches can >> be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal >> >> If your patch is accepted and if you do not have write-access to the >> repository, one of the maintainers will likely take care of it. >> If you become a regular contributor, you will probably want to consider >> getting write access. >> >> Cheers, >> Harald >> >> >> >> On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote: >> > Hello, >> > >> > I am new to the GFortran community. Over the past two weeks I created a >> > patch that should fix PR82943 for GFortran. I have attached it to this >> > email. The patch allows the code below to compile successfully. I am >> > working on creating test cases next, but I am new to the process so it >> may >> > take me some time. After I make test cases, do I email them to you as >> well? >> > Do I need to make a pull-request on github in order to get the patch >> > reviewed? >> > >> > Thank you, >> > >> > Alexander Westbrooks >> > >> > module testmod >> > >> > public :: foo >> > >> > type, public :: tough_lvl_0(a, b) >> > integer, kind :: a = 1 >> > integer, len :: b >> > contains >> > procedure :: foo >> > end type >> > >> > type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c) >> > integer, len :: c >> > contains >> > procedure :: bar >> > end type >> > >> > type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d) >> > integer, len :: d >> > contains >> > procedure :: foobar >> > end type >> > >> > contains >> > subroutine foo(this) >> > class(tough_lvl_0(1,*)), intent(inout) :: this >> > end subroutine >> > >> > subroutine bar(this) >> > class(tough_lvl_1(1,*,*)), intent(inout) :: this >> > end subroutine >> > >> > subroutine foobar(this) >> > class(tough_lvl_2(1,*,*,*)), intent(inout) :: this >> > end subroutine >> > >> > end module >> > >> > PROGRAM testprogram >> > USE testmod >> > >> > TYPE(tough_lvl_0(1,5)) :: test_pdt_0 >> > TYPE(tough_lvl_1(1,5,6)) :: test_pdt_1 >> > TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2 >> > >> > CALL test_pdt_0%foo() >> > >> > CALL test_pdt_1%foo() >> > CALL test_pdt_1%bar() >> > >> > CALL test_pdt_2%foo() >> > CALL test_pdt_2%bar() >> > CALL test_pdt_2%foobar() >> > >> > >> > END PROGRAM testprogram >> >>
[committed] OpenMP/Fortran: Parsing support for 'uses_allocators'
Committed the attached patch as r14-2582-g89d0f082b3c95f.
This is about OpenMP's uses_allocators clause to the 'target' directive.
Using the clause with predefined allocators as list arguments is
required if those allocators are used in a target region - unless
there is an 'omp requires dynamic_allocators' in the compilation unit.
While the later is a no op (requirement fulfilled by all devices), we
still had to handle the no op when using 'uses_allocators', which this
commit does.
However, uses_allocators also permits to define new allocators; for
those, this commit stops after parsing and resolving with a
'sorry, unimplemented'.
Support for the latter will be added together with the C/C++ support
by a re-diffed/updated version of Chung-Lin's patch at
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596587.html
(See thread for pending review issues; the C++ member var issue
is https://gcc.gnu.org/PR110347 )
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
commit 89d0f082b3c95f68d116d4480126e3ab7fb7f36b
Author: Tobias Burnus
Date: Mon Jul 17 15:13:44 2023 +0200
OpenMP/Fortran: Parsing support for 'uses_allocators'
The 'uses_allocators' clause to the 'target' construct accepts predefined
allocators and can also be used to define a new allocator for a target region.
As predefined allocators in GCC do not require special handling, those can and
are ignored after parsing, such that this feature now works. On the other hand,
defining a new allocator will fail for now with a 'sorry, unimplemented'.
Note that both the OpenMP 5.0/5.1 and 5.2 syntax for uses_allocators
is supported by this commit.
2023-07-17 Tobias Burnus
Chung-Lin Tang
gcc/fortran/ChangeLog:
* dump-parse-tree.cc (show_omp_namelist, show_omp_clauses): Dump
uses_allocators clause.
* gfortran.h (gfc_free_omp_namelist): Add memspace_sym to u union
and traits_sym to u2 union.
(OMP_LIST_USES_ALLOCATORS): New enum value.
(gfc_free_omp_namelist): Add 'bool free_mem_traits_space' arg.
* match.cc (gfc_free_omp_namelist): Likewise.
* openmp.cc (gfc_free_omp_clauses, gfc_match_omp_variable_list,
gfc_match_omp_to_link, gfc_match_omp_doacross_sink,
gfc_match_omp_clause_reduction, gfc_match_omp_allocate,
gfc_match_omp_flush): Update call.
(gfc_match_omp_clauses): Likewise. Parse uses_allocators clause.
(gfc_match_omp_clause_uses_allocators): New.
(enum omp_mask2): Add new OMP_CLAUSE_USES_ALLOCATORS.
(OMP_TARGET_CLAUSES): Accept it.
(resolve_omp_clauses): Resolve uses_allocators clause
* st.cc (gfc_free_statement): Update gfc_free_omp_namelist call.
* trans-openmp.cc (gfc_trans_omp_clauses): Handle
OMP_LIST_USES_ALLOCATORS; fail with sorry unless predefined allocator.
(gfc_split_omp_clauses): Handle uses_allocators.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/uses_allocators_1.f90: New test.
* testsuite/libgomp.fortran/uses_allocators_2.f90: New test.
Co-authored-by: Chung-Lin Tang
---
gcc/fortran/dump-parse-tree.cc | 24 +++
gcc/fortran/gfortran.h | 5 +-
gcc/fortran/match.cc | 7 +-
gcc/fortran/openmp.cc | 194 +++--
gcc/fortran/st.cc | 2 +-
gcc/fortran/trans-openmp.cc| 11 ++
.../libgomp.fortran/uses_allocators_1.f90 | 168 ++
.../libgomp.fortran/uses_allocators_2.f90 | 99 +++
8 files changed, 491 insertions(+), 19 deletions(-)
diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc
index effcebe9325..68122e3e6fd 100644
--- a/gcc/fortran/dump-parse-tree.cc
+++ b/gcc/fortran/dump-parse-tree.cc
@@ -1497,6 +1497,29 @@ show_omp_namelist (int list_type, gfc_omp_namelist *n)
case OMP_LINEAR_UVAL: fputs ("uval(", dumpfile); break;
default: break;
}
+ else if (list_type == OMP_LIST_USES_ALLOCATORS)
+ {
+ if (n->u.memspace_sym)
+ {
+ fputs ("memspace(", dumpfile);
+ fputs (n->sym->name, dumpfile);
+ fputc (')', dumpfile);
+ }
+ if (n->u.memspace_sym && n->u2.traits_sym)
+ fputc (',', dumpfile);
+ if (n->u2.traits_sym)
+ {
+ fputs ("traits(", dumpfile);
+ fputs (n->u2.traits_sym->name, dumpfile);
+ fputc (')', dumpfile);
+ }
+ if (n->u.memspace_sym || n->u2.traits_sym)
+ fputc (':', dumpfile);
+ fputs (n->sym->name, dumpfile);
+
Re: [PATCH 00/14] fortran: Use precalculated class container for deallocation [PR110618]
Hi Mikhail, That's ever so slightly embarrassing :-) My notes for that commit don't provide any enlightenment. Thanks Paul
