Re: Team Collaboration Considerations
On Thu, Jan 19, 2023, 00:01 Benson Muite via Fortran wrote: > The GCC workflows is quite different from other open source projects > being primarily email based and not using a bug tracker. > There's a bug tracker. I think you mean there isn't a patch tracker (or at least not a system where you submit patches to a thing vs emailing them to a list and hoping someone replies). >
Re: Team Collaboration Considerations
On 1/19/23 13:28, NightStrike via Fortran wrote: On Thu, Jan 19, 2023, 00:01 Benson Muite via Fortran wrote: The GCC workflows is quite different from other open source projects being primarily email based and not using a bug tracker. There's a bug tracker. I think you mean there isn't a patch tracker (or at least not a system where you submit patches to a thing vs emailing them to a list and hoping someone replies). Well, you can put patches into Bugzilla ... Kind regards, -- Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290 Saturnushof 14, 3738 XG Maartensdijk, The Netherlands
Re: Team Collaboration Considerations
On Thu, Jan 19, 2023 at 7:46 AM Toon Moene wrote: > > On 1/19/23 13:28, NightStrike via Fortran wrote: > > > On Thu, Jan 19, 2023, 00:01 Benson Muite via Fortran > > wrote: > > > >> The GCC workflows is quite different from other open source projects > >> being primarily email based and not using a bug tracker. > >> > > > > There's a bug tracker. I think you mean there isn't a patch tracker (or at > > least not a system where you submit patches to a thing vs emailing them to > > a list and hoping someone replies). > Well, you can put patches into Bugzilla ... You can, and people naturally do this, and I think it's great, but there's usually a response from someone saying "post that to the mailing list instead".
[Patch] OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]
This is all about non-rectangular loop nests in OpenMP.
The attached patch depends on the obvious fix for https://gcc.gnu.org/PR108459,
which is together with a nice testcase in Jakub's WIP patch attached to the PR;
without, gfortran.dg/gomp/canonical-loop-1.f90 fails with an ICE (segfault).
My patch fixes part of the Fortran issues found. Namely, it ensures that a
"regular"
non-rectangular loop nest actually works by passing the outer-loop-var, the
multiplier
and offset in a TREE_VEC to the middle end. It additionally avoids pointlessly
creating a temporary variable for a VAR_DECL (main advantage: dump looks
cleaner and
avoids some dependency analysis) - and likewise for 'step' given that 'step' was
evaluated before.
There is an additional issue - not quite addressed in this patch: There are
cases
when a loop variable is replaced by another variable ('count') and then at the
beginning
of the loop body, the original variable gets the value from the count variable.
Obviously,
this no longer works with non-rectangular loop nests.
The 'count' appears in two cases: (a) when the iteration step is not 1 or -1
and (b) if
the iteration variable is a pointer (scalar with allocatable, pointer, optional
argument
or just a dummy argument; oddly, even if it has the value attribute).
There is pending work to be done in this case, as mentioned in comment 6 and 8
of the PR.
This patch adds some 'sorry' messages for them. I hope and think that I have
not missed
a case where 'count' is used which I did not catch, but I should have all or at
least most.
OK for mainline, once the other patch has been committed?
Tobias
PS: I still need to verify that everything is fine, once the other patch has
been committed.
A flaky mainboard on the laptop causes multiple random freezes per day, which
makes testing
+ patch writing a bit harder. (At least the mainboard replacement is scheduled
for tomorrow :-) )
-
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
OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]
This patch ensures that loop bounds depending on outer loop vars use the
proper TREE_VEC format. It additionally gives a sorry if such an outer
var has a non-one/non-minus-one increment as currently a count variable
is used in this case (see PR).
gcc/fortran/ChangeLog:
PR fortran/107424
* trans-openmp.cc (gfc_nonrect_loop_expr): New.
(gfc_trans_omp_do): Call it for start/end loop bound
for non-rectangular loop nests.
gcc/testsuite/
PR fortran/107424
* gfortran.dg/gomp/non-rectangular-loop-3.f90: New test.
libgomp/ChangeLog:
PR fortran/107424
* testsuite/libgomp.fortran/non-rectangular-loop-1.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-1a.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-2.f90: New test.
gcc/fortran/trans-openmp.cc| 167 +-
.../gfortran.dg/gomp/non-rectangular-loop-3.f90| 85 +++
.../libgomp.fortran/non-rectangular-loop-1.f90 | 637 +
.../libgomp.fortran/non-rectangular-loop-1a.f90| 374
.../libgomp.fortran/non-rectangular-loop-2.f90 | 243
5 files changed, 1495 insertions(+), 11 deletions(-)
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index 87213de0918..73376894316 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -5120,6 +5120,136 @@ typedef struct dovar_init_d {
tree init;
} dovar_init;
+static bool
+gfc_nonrect_loop_expr (stmtblock_t *pblock, gfc_se *sep, int loop_n,
+ gfc_code *code, gfc_expr *expr, vec *inits)
+{
+ int i;
+ for (i = 0; i < loop_n; i++)
+{
+ gcc_assert (code->ext.iterator->var->expr_type == EXPR_VARIABLE);
+ if (gfc_find_sym_in_expr (code->ext.iterator->var->symtree->n.sym, expr))
+ break;
+ code = code->block->next;
+}
+ if (i >= loop_n)
+return false;
+
+ /* Canonic format: TREE_VEC with [var, multiplier, offset]. */
+ gfc_symbol *var = code->ext.iterator->var->symtree->n.sym;
+
+ gfc_se se;
+ tree tree_var, a1, a2;
+ a1 = integer_one_node;
+ a2 = integer_zero_node;
+
+ gfc_init_se (&se, NULL);
+ gfc_conv_expr_lhs (&se, code->ext.iterator->var);
+ gfc_add_block_to_block (pblock, &se.pre);
+ tree_var = se.expr;
+
+ {
+/* FIXME: Handle non-unity iterations, cf. PR fortran/107424.
+ The issue is that for those a 'count' variable is used. */
+dovar_init *di;
+unsigned ix;
+tree t = tree_var;
+while (TREE_CODE (t) == INDIRECT_REF)
+ t = TREE_OPERAND (t, 0);
+FOR_EACH_VEC_ELT (*inits, ix, di)
+ {
+ tree t2 = di->var;
+ while (TREE_CODE (t2) == INDIRECT_REF)
+ t2 = TREE_OPERAND (t2, 0);
+ if (t == t2)
+ {
+ HOST_WIDE_INT intval;
+ if (gfc_extract_hwi (code->ext.it
Re: Team Collaboration Considerations
On 19 January 2023 13:52:55 CET, NightStrike via Fortran wrote: >You can, and people naturally do this, and I think it's great, but >there's usually a response from someone saying "post that to the >mailing list instead". The mailing list has a 20-30 year history with reasoning about what currently is in the tree. I do think it is valuable to reason about patches publically for others to see. And I'm aware that this might not be regarded fancy nowadays by everyone. But that does not mean that using other means to collaborate should not be used by some. Be it comp.lang.fortran, a webchat.oftc, or other means, that's all fine of course. patches currently are handled differently, but I don't think that is a problem isn't it. Just post final patches to the list as long as that is regarded the way to do final review and document approval. cheers,
Re: Team Collaboration Considerations
On Thu, Jan 19, 2023, 13:33 Bernhard Reutner-Fischer wrote: > On 19 January 2023 13:52:55 CET, NightStrike via Fortran < > fortran@gcc.gnu.org> wrote: > > >You can, and people naturally do this, and I think it's great, but > >there's usually a response from someone saying "post that to the > >mailing list instead". > > The mailing list has a 20-30 year history with reasoning about what > currently is in the tree. I do think it is valuable to reason about patches > publically for others to see. And I'm aware that this might not be regarded > fancy nowadays by everyone. > > But that does not mean that using other means to collaborate should not be > used by some. Be it comp.lang.fortran, a webchat.oftc, or other means, > that's all fine of course. > > patches currently are handled differently, but I don't think that is a > problem isn't it. > Just post final patches to the list as long as that is regarded the way to > do final review and document approval. > > cheers, > The problem is that patch tracking is unsustainable. You could go the other way and have a patch tracker automatically echo messages to the mailing list. >
git out-of-order commit (was Re: [PATCH] Fortran: Remove unused declaration)
On Sat, Nov 12, 2022 at 4:24 PM Harald Anlauf via Gcc-patches wrote: > > Am 12.11.22 um 22:05 schrieb Bernhard Reutner-Fischer via Gcc-patches: > > This function definition was removed years ago, remove it's prototype. > > > > gcc/fortran/ChangeLog: > > > > * gfortran.h (gfc_check_include): Remove declaration. > > --- > > gcc/fortran/gfortran.h | 1 - > > 1 file changed, 1 deletion(-) > > --- > > Regtests cleanly, ok for trunk? > > > > diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h > > index c4deec0d5b8..ce3ad61bb52 100644 > > --- a/gcc/fortran/gfortran.h > > +++ b/gcc/fortran/gfortran.h > > @@ -3208,7 +3208,6 @@ int gfc_at_eof (void); > > int gfc_at_bol (void); > > int gfc_at_eol (void); > > void gfc_advance_line (void); > > -int gfc_check_include (void); > > int gfc_define_undef_line (void); > > > > int gfc_wide_is_printable (gfc_char_t); > > OK, thanks. Somehow this was applied with a CommitDate in 2021, breaking scripts that assume monotonically increasing CommitDate. Anyone know how that could have happened? Jason
Chained modules in a testcase
Hi, Andrew Benson found a rather horrible bug in my finalization patch that involved two separate module files and another containing both a module and the main program. They must be compiled separately for the bug to appear. This bug arises in finalization because some derived type function results wind up with a derived type that has no proc_name for its namespace, which causes gfc_get_derived_vtab to segfault. file 1: iso.f90 which contains a module only. file 2: dt.90 which uses module iso within one of the module contained functions file 3: ov.f90 which contains another module using iso and dt, again within a module constrained function. It also contains the main program. How do I deal with this in the testsuite? I have tried all manner of combinations of dg-compile-aux-modules and dg-additional-sources to no avail. Cheers Paul
Re: git out-of-order commit (was Re: [PATCH] Fortran: Remove unused declaration)
Am 19.01.23 um 20:39 schrieb Jason Merrill via Gcc-patches: On Sat, Nov 12, 2022 at 4:24 PM Harald Anlauf via Gcc-patches wrote: Am 12.11.22 um 22:05 schrieb Bernhard Reutner-Fischer via Gcc-patches: This function definition was removed years ago, remove it's prototype. gcc/fortran/ChangeLog: * gfortran.h (gfc_check_include): Remove declaration. --- gcc/fortran/gfortran.h | 1 - 1 file changed, 1 deletion(-) --- Regtests cleanly, ok for trunk? diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index c4deec0d5b8..ce3ad61bb52 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3208,7 +3208,6 @@ int gfc_at_eof (void); int gfc_at_bol (void); int gfc_at_eol (void); void gfc_advance_line (void); -int gfc_check_include (void); int gfc_define_undef_line (void); int gfc_wide_is_printable (gfc_char_t); OK, thanks. Somehow this was applied with a CommitDate in 2021, breaking scripts that assume monotonically increasing CommitDate. Anyone know how that could have happened? It is quite unusual that the CommitDate is before the AuthorDate: % git show --pretty=fuller 7ce0cee77adf33397d0ba61e7445effd8a5d8fcc | head -5 commit 7ce0cee77adf33397d0ba61e7445effd8a5d8fcc Author: Bernhard Reutner-Fischer AuthorDate: Sat Nov 6 06:51:00 2021 +0100 Commit: Bernhard Reutner-Fischer CommitDate: Sat Nov 6 06:48:00 2021 +0100 Could this have prevented checks to work properly? Harald Jason
Re: Chained modules in a testcase
Hi Paul, I got a list of potential candidates by % grep 'dg-additional-sources .*f90 ' gcc/testsuite/gfortran.dg/ -r and ran the first one: % make check-fortran RUNTESTFLAGS='dg.exp=altreturn_9*.*' On first sight this looked fine. Cheers, Harald
Re: git out-of-order commit (was Re: [PATCH] Fortran: Remove unused declaration)
On 19 January 2023 20:39:08 CET, Jason Merrill wrote: >On Sat, Nov 12, 2022 at 4:24 PM Harald Anlauf via Gcc-patches > wrote: >> >> Am 12.11.22 um 22:05 schrieb Bernhard Reutner-Fischer via Gcc-patches: >> > This function definition was removed years ago, remove it's prototype. >> > >> > gcc/fortran/ChangeLog: >> > >> > * gfortran.h (gfc_check_include): Remove declaration. >> > --- >> > gcc/fortran/gfortran.h | 1 - >> > 1 file changed, 1 deletion(-) >> > --- >> > Regtests cleanly, ok for trunk? >> > >> > diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h >> > index c4deec0d5b8..ce3ad61bb52 100644 >> > --- a/gcc/fortran/gfortran.h >> > +++ b/gcc/fortran/gfortran.h >> > @@ -3208,7 +3208,6 @@ int gfc_at_eof (void); >> > int gfc_at_bol (void); >> > int gfc_at_eol (void); >> > void gfc_advance_line (void); >> > -int gfc_check_include (void); >> > int gfc_define_undef_line (void); >> > >> > int gfc_wide_is_printable (gfc_char_t); >> >> OK, thanks. > >Somehow this was applied with a CommitDate in 2021, breaking scripts >that assume monotonically increasing CommitDate. Anyone know how that >could have happened? Sorry for that. I think i cherry-picked this commit to master before pushing it, not 100% sure though. What shall we do now?
Re: Team Collaboration Considerations
On 19 January 2023 20:03:38 CET, NightStrike wrote: >The problem is that patch tracking is unsustainable. You could go the other >way and have a patch tracker automatically echo messages to the mailing >list. Currently it's the other way round. Patchwork collects the patches sent to the list. Everybody is free to administrate her patches in Patchwork iff that is desired. Or do the same but through bugzilla. I am aware that all this might not be the way you envision things to work, of course.
Re: Team Collaboration Considerations
On 1/19/23 22:03, NightStrike wrote: > > > On Thu, Jan 19, 2023, 13:33 Bernhard Reutner-Fischer > mailto:rep.dot@gmail.com>> wrote: > > On 19 January 2023 13:52:55 CET, NightStrike via Fortran > mailto:fortran@gcc.gnu.org>> wrote: > > >You can, and people naturally do this, and I think it's great, but > >there's usually a response from someone saying "post that to the > >mailing list instead". > > The mailing list has a 20-30 year history with reasoning about what > currently is in the tree. I do think it is valuable to reason about > patches publically for others to see. And I'm aware that this might > not be regarded fancy nowadays by everyone. > > But that does not mean that using other means to collaborate should > not be used by some. Be it comp.lang.fortran, a webchat.oftc, or > other means, that's all fine of course. > > patches currently are handled differently, but I don't think that is > a problem isn't it. > Just post final patches to the list as long as that is regarded the > way to do final review and document approval. > > cheers, > > > The problem is that patch tracking is unsustainable. You could go the > other way and have a patch tracker automatically echo messages to the > mailing list. > Review is done voluntarily and by email. The process has flexibility to increase productivity but it may make it harder to get involved. Email threads and metadata make it possible to track patches. It is expected, but not enforced, that posts to the mailing list follow convention. Until a comment on a patch is posted, it is unclear if it will be reviewed. Build results are tracked separately and builds done when needed. An open review process is good. Do all patches get timely reviews? Can this process be improved? An analysis of a developer community: https://carlschwan.eu/2021/04/29/health-of-the-kde-community/ Some tools: https://chaoss.github.io/grimoirelab/ https://framagit.org/ervin/ComDaAn https://github.com/gotec/git2net Will try some of these to analyze GCC, though something new maybe needed since the workflow is quite different.
