Re: [PATCH 1/2] OpenMP/Fortran: Combined directives with map/firstprivate of same symbol
On 07.12.22 20:09, Julian Brown wrote: On Wed, 26 Oct 2022 12:39:39 +0200 Tobias Burnus wrote: The ICE seems to be because gcc/fortran/trans-openmp.cc's gfc_split_omp_clauses mishandles this as the dump shows the following: #pragma omp target firstprivate(a) map(tofrom:a) #pragma omp parallel firstprivate(a) In contrast, for the C testcase: #pragma omp target parallel for simd map(x) firstprivate(x) the dump is as follows, which seems to be sensible: #pragma omp target map(tofrom:x) #pragma omp parallel firstprivate(x) This patch fixes a case where a combined directive (e.g. "!$omp target parallel ...") contains both a map and a firstprivate clause for the same variable. When the combined directive is split into two nested directives, the outer "target" gets the "map" clause, and the inner "parallel" gets the "firstprivate" clause, like so: ... This is not a recent regression, but appears to fix a long-standing ICE. ... gcc/fortran/ * trans-openmp.cc (gfc_add_firstprivate_if_unmapped): New function. (gfc_split_omp_clauses): Call above. libgomp/ * testsuite/libgomp.fortran/combined-directive-splitting-1.f90: New test. LGTM – thanks! 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] tree-optimization/99919 - bogus uninit diagnostic with bitfield guards
For the testcase in this PR what fold-const.cc optimize_bit_field_compare
does to bitfield against constant compares is confusing the uninit
predicate analysis and it also makes SRA obfuscate instead of optimize
the code. We've long had the opinion that those optimizations are
premature but we do not have any replacement for the more complicated
ones combining multiple bitfield tests. The following disables mangling
the case of a single bitfield test against constants but preserving
the existing diagnostic and optimization to a compile-time determined
value.
This requires silencing a bogus uninit diagnostic in the Fortran
frontend which I've done in a minimal way, avoiding initializing
the 40 byte symbol_attribute structure. There's several issues,
one is the flag_coarrays is a global variable likely not CSEd
to help the uninit predicate analysis, the other is us short-circuiting
the flag_coarray == GFC_FCOARRAY_LIB && lhs_caf_attr.codimension
accesses as both have no side-effects so the guard isn't effective.
If the frontend folks are happy with this I can localize both
lhs_caf_attr and rhs_caf_attr and copy out the two only flags
tested by the code instead of the somewhat incomplete approach in
the patch. Any opinions here?
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
OK for the fortran parts?
Thanks,
Richard.
PR tree-optimization/99919
* fold-const.cc (optimize_bit_field_compare): Disable
transforming the bitfield against constant compare optimization
if the result is not statically determinable.
gcc/fortran/
* trans-expr.cc (gfc_trans_assignment_1): Split out
lhs_codimension from lhs_caf_attr to avoid bogus uninit
diagnostics.
* gcc.dg/uninit-pr99919.c: New testcase.
---
gcc/fold-const.cc | 37 +++
gcc/fortran/trans-expr.cc | 6 +++--
gcc/testsuite/gcc.dg/uninit-pr99919.c | 22
3 files changed, 30 insertions(+), 35 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/uninit-pr99919.c
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index cdfe3f50ae3..b72cc0a1d51 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -4559,7 +4559,6 @@ optimize_bit_field_compare (location_t loc, enum
tree_code code,
{
poly_int64 plbitpos, plbitsize, rbitpos, rbitsize;
HOST_WIDE_INT lbitpos, lbitsize, nbitpos, nbitsize;
- tree type = TREE_TYPE (lhs);
tree unsigned_type;
int const_p = TREE_CODE (rhs) == INTEGER_CST;
machine_mode lmode, rmode;
@@ -4667,13 +4666,7 @@ optimize_bit_field_compare (location_t loc, enum
tree_code code,
}
/* Otherwise, we are handling the constant case. See if the constant is too
- big for the field. Warn and return a tree for 0 (false) if so. We do
- this not only for its own sake, but to avoid having to test for this
- error case below. If we didn't, we might generate wrong code.
-
- For unsigned fields, the constant shifted right by the field length should
- be all zero. For signed fields, the high-order bits should agree with
- the sign bit. */
+ big for the field. Warn and return a tree for 0 (false) if so. */
if (lunsignedp)
{
@@ -4695,31 +4688,9 @@ optimize_bit_field_compare (location_t loc, enum
tree_code code,
}
}
- if (nbitpos < 0)
-return 0;
-
- /* Single-bit compares should always be against zero. */
- if (lbitsize == 1 && ! integer_zerop (rhs))
-{
- code = code == EQ_EXPR ? NE_EXPR : EQ_EXPR;
- rhs = build_int_cst (type, 0);
-}
-
- /* Make a new bitfield reference, shift the constant over the
- appropriate number of bits and mask it with the computed mask
- (in case this was a signed field). If we changed it, make a new one. */
- lhs = make_bit_field_ref (loc, linner, lhs, unsigned_type,
- nbitsize, nbitpos, 1, lreversep);
-
- rhs = const_binop (BIT_AND_EXPR,
-const_binop (LSHIFT_EXPR,
- fold_convert_loc (loc, unsigned_type, rhs),
- size_int (lbitpos)),
-mask);
-
- lhs = build2_loc (loc, code, compare_type,
- build2 (BIT_AND_EXPR, unsigned_type, lhs, mask), rhs);
- return lhs;
+ /* Otherwise do not prematurely optimize compares of bitfield members
+ to constants. */
+ return 0;
}
/* Subroutine for fold_truth_andor_1: decode a field reference.
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index b95c5cf2f96..12c7dd7f26a 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -11654,9 +11654,11 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr *
expr2, bool init_flag,
/* Only analyze the expressions for coarray properties, when in coarray-lib
mode. */
+ bool lhs_codimension = false;
if (flag_coarray == GFC_FCOARRAY_LIB)
{
lhs_caf_attr = gfc_caf_attr (expr1,
Re: [PATCH 2/2] OpenMP: Duplicate checking for map clauses in Fortran (PR107214)
Hi Julian:
On 07.12.22 20:13, Julian Brown wrote:
I know that this was the case before, but can you move the mark:1 etc.
after 'tlink'? In that case all bitfields are grouped together.
Thanks for doing so.
I wonder whether that also rejects the following – which seems to be
valid. The 'map' goes to 'target' and the 'firstprivate' to
'parallel', cf. OpenMP 5.2, "17.2 Clauses on Combined and Composite
Constructs", [340:3-4 & 12-14]. (BTW: While some fixes went into 5.1
regarding this section, a likewise wording is already in 5.0.)
(Testing showed: it give an ICE without the patch and an error with.)
...and this patch avoids the error for combined directives, and
reorders the gfc_symbol bitfields.
All in all, I am fine with the patch - but I spotted some issues.
First, I think you need to set for some error cases mark = 0 to avoid
duplicated errors.
Namely:
! Outputs the error twice ('Symbol ‘y’ present on multiple clauses')
!$omp target has_device_addr(y) firstprivate(y)
block; end block
* * *
Additionally, I think it would be good to have besides 'target' +
map/firstprivate (→ error)
also a testcase for 'target simd' + map/firstprivate → error
And I think also gives-no-error checks all combined 'target ...' that take
firstprivate
should be added, cf. your own patch - possibly with looking at the original
dump (scan-tree-dump)
to see that the clause is properly attached correctly. Example for 'target
teams':
!$omp target teams map(x) firstprivate(x)
block; end block
(Works but no testcase.)
* * *
The following is not diagnosed and gives an ICE:
!$omp target in_reduction(+: x) private(x)
block; end block
end
The C testcase properly has:
error: ‘x’ appears more than once in data-sharing clauses
Note: Using 'firstprivate' instead of 'private' shows the proper error also in
Fortran.
The following does not ICE but does not make sense (and is rejected in C):
4 | #pragma omp target private(x) map(x)
vs.
!$omp target map(x) private(x)
block; end block
(The latter produces "#pragma omp target private(x.0) map(tofrom:*x.0)", ups!)
* * *
I also note that 'simd' accepts private such that
#pragma omp target simd private(x) map(x)
for (int i=0; i < 0; i++)
;
!$omp target simd map(x) private(x)
do i = 1, 0; end do
is valid. (It is accepted by gcc and gfortran, i.e. it just needs to be added
as testcase.)
* * *
I note that C rejects {map(x),firstprivate(x)} +
{has_device_addr(x),is_device_ptr(x)}',
but gfortran + your patch accepts:
!$omp target map(x) has_device_addr(x)
!$omp target map(x) is_device_ptr(x)
while
!$omp target firstprivate(x) has_device_addr(x)
!$omp target firstprivate(x) is_device_ptr(x)
is rejected – showing the error message twice.
Expected: I think it should show an error in all four cases - but only once.
2022-12-06 Julian Brown
gcc/fortran/
PR fortran/107214
* gfortran.h (gfc_symbol): Add data_mark, dev_mark, gen_mark and
reduc_mark bitfields.
* openmp.cc (resolve_omp_clauses): Use above bitfields to improve
duplicate clause detection.
gcc/testsuite/
PR fortran/107214
* gfortran.dg/gomp/pr107214.f90: New test.
Thanks,
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: Team Collaboration Considerations
On Wed, Dec 07, 2022 at 05:54:40PM -0800, Jerry D via Fortran wrote: > Other than Benson, I have received no sign of any interest from gfortran > developers to adopt a teaming/collaboration platform. I am a bit > disappointed. Maybe my intent was misunderstood. I am not suggesting > replacing the email approval process but there are many other features of > these platforms, in particular, communication efficiency that would be very > helpful. > > I know many software developers who use these tools regularly. Honestly I do > not know why gcc.gnu.org does not adopt one of these as a whole. Perhaps it > is simply resistance to change. > > I will keep the Mattermost workspace. If anyone want to join send me your > email and I will send you an invite. > > Well, as always, best regards, > Jerry, I think you're seeing the effects of the move to git and an aging base of contributors. Harald is almost single-handily dealing with bug reports. Mikeal has recently been reviewing Harald's patches, and offers some good advice on improvements. Occasionally, Thomas and I offer up patches, but this occurs in a rather sparse manner. In fact, if I fix a bug, the patch is attached to the bugzilla report where it sits until Harald stumbles across it or it bit rots. I don't know how to attract new contributors. I've invited more than one person who's pointed out a issue with gfortran to join the developers. This is typically met with "I don't know C", "I don't know compiler design", "I don't have time"r, ... -- Steve
Re: Team Collaboration Considerations
Hi, On 08.12.22 17:27, Steve Kargl via Fortran wrote: On Wed, Dec 07, 2022 at 05:54:40PM -0800, Jerry D via Fortran wrote: Other than Benson, I have received no sign of any interest from gfortran developers to adopt a teaming/collaboration platform. I am a bit disappointed. Maybe my intent was misunderstood. I am not suggesting replacing the email approval process but there are many other features of these platforms, in particular, communication efficiency that would be very helpful. I personally do not have a strong preference for any. But every additional communication channel costs time time (setting up, tracking discussions etc.) and it is not really clear what's the benefits vs. costs ratio. I don't mind adding yet another channel to watch, but I cannot guarantee that I will be able to actively follow it. If it is not high volume and I get some notification that something has changed by email, it might work – or if it is interesting/often enough used, I might log in from time to time – but otherwise it will simply get ignored. Not due to not being interested but due to having too much else on to do. I will keep the Mattermost workspace. If anyone want to join send me your email and I will send you an invite. Let's try it – please send an invite. I think you're seeing the effects of the move to git and an aging base of contributors. I don't think that moving to 'git' really causes the problem – nor the move to C++, given that is is not really visible in most code. I think the main problem is that most things do work and attracting a new contributor is always difficult. Google Summer of Code was one successful way, but attracting contributors is not trivial. This year, two were interested working on gfortran but at the end it did not work out – despite me spending quite some time to guide them to get started. Occasionally, Thomas and I offer up patches, but this occurs in a rather sparse manner. In fact, if I fix a bug, the patch is attached to the bugzilla report where it sits until Harald stumbles across it or it bit rots. ... "I don't have time" I think we have three areas: bug fixes (where Harald does an awesome job), cleanup/restructuring of some bad design, and new features (mainly but not only F2018). The problem with the second item is that it takes quite some work for little visible benefit; we do some bits here and there for bug fixes (or for TS29113) but nothing larger. Likewise for F2018, some low-hanging fruits are easily done, but some larger development work is really due. I still intend to do more on the gfortran side, but I am rather busy on the OpenMP side (code, specification meetings including filing issues and creating spec patches) and things like working on gfortran competes with fixing other GCC issues (a pragma, documentation, long-standing other Bugzilla bugs), fixing issues in external testsuites/libraries, GSoC mentoring, internal mentoring etc. I still hope that I will do more non-OpenMP gfortran work but it does not look as if I will do a lot any time soon. 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: Team Collaboration Considerations
I was thinking I might try to contribute when I retire, though that may be in a year or two. But it's been a very long time since I dove into a large software project and it's intimidating. I do know C (really C++, I haven't used plain C for a long time). I am one of those "aging" types but I am familiar at least superficially with newer tools because I must use them for work, specifically git and Slack (Mattermost seems to be an open-source Slack alternative) -- we make heavy use of Slack in particular. Is there some kind of "getting started" guide? Katherine Holcomb UVA Research Computing https://www.rc.virginia.edu ka...@virginia.edu434-982-5948 -Original Message- From: Fortran On Behalf Of Steve Kargl via Fortran Sent: Thursday, December 8, 2022 11:27 AM To: Jerry D via Fortran Cc: Benson Muite Subject: Re: Team Collaboration Considerations On Wed, Dec 07, 2022 at 05:54:40PM -0800, Jerry D via Fortran wrote: > Other than Benson, I have received no sign of any interest from > gfortran developers to adopt a teaming/collaboration platform. I am a > bit disappointed. Maybe my intent was misunderstood. I am not > suggesting replacing the email approval process but there are many > other features of these platforms, in particular, communication > efficiency that would be very helpful. > > I know many software developers who use these tools regularly. > Honestly I do not know why gcc.gnu.org does not adopt one of these as > a whole. Perhaps it is simply resistance to change. > > I will keep the Mattermost workspace. If anyone want to join send me > your email and I will send you an invite. > > Well, as always, best regards, > Jerry, I think you're seeing the effects of the move to git and an aging base of contributors. Harald is almost single-handily dealing with bug reports. Mikeal has recently been reviewing Harald's patches, and offers some good advice on improvements. Occasionally, Thomas and I offer up patches, but this occurs in a rather sparse manner. In fact, if I fix a bug, the patch is attached to the bugzilla report where it sits until Harald stumbles across it or it bit rots. I don't know how to attract new contributors. I've invited more than one person who's pointed out a issue with gfortran to join the developers. This is typically met with "I don't know C", "I don't know compiler design", "I don't have time"r, ... -- Steve
Team Collaboration Considerations
Hi Jerry, all, as already said, the basic issue appears to be the low number of contributors, most (all?) of which are working on gfortran in their spare time (like me). I think it is not likely that they will commit to being available regularly. I have used IRC once for a discussion that was really helpful for me, but we agreed in advance to meet there, and this was how it worked. Other tools may work similar or even better. (The ML was just not appropriate for the type of discussion.) A collaboration tool could be helpful for introducing newcomers, for giving advice in trying to understand how the code works, and maybe when it doesn't. Caveat: in many respects I still feel as close to being a newcomer. Though I could give a very basic introduction to working with git, also in the context of gcc, if that is needed... ;-) (Perhaps using git even is more fun than programming in C :-) Personally, many things work fine for me using bugzilla and the mailing list. I just don't expect immediate replies because of the current situation. So can we get the attention of new possible contributors? Or increase the activity of some of those who've been contributing in the past? So that a critical mass is reached for a modern collaboration tool to be useful? Cheers, Harald
[PATCH] Fortran: diagnose and reject duplicate CONTIGUOUS attribute [PR108025]
Dear all,
a fairly obvious, or rather trivial fix that appeared while
analyzing another pr and that can be treated independently:
reject duplicate CONTIGUOUS attributes.
(Intel and NAG reject this, Cray warns that this is non-standard.)
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Thanks,
Harald
From 3a9f6d5a8ee490adf9a18f93feaf86542642be7d Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Thu, 8 Dec 2022 22:50:45 +0100
Subject: [PATCH] Fortran: diagnose and reject duplicate CONTIGUOUS attribute
[PR108025]
gcc/fortran/ChangeLog:
PR fortran/108025
* symbol.cc (gfc_add_contiguous): Diagnose and reject duplicate
CONTIGUOUS attribute.
gcc/testsuite/ChangeLog:
PR fortran/108025
* gfortran.dg/contiguous_12.f90: New test.
---
gcc/fortran/symbol.cc | 6 ++
gcc/testsuite/gfortran.dg/contiguous_12.f90 | 7 +++
2 files changed, 13 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/contiguous_12.f90
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 49fb37864bd..e704e7ac2bd 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -1108,6 +1108,12 @@ gfc_add_contiguous (symbol_attribute *attr, const char *name, locus *where)
if (check_used (attr, name, where))
return false;
+ if (attr->contiguous)
+{
+ duplicate_attr ("CONTIGUOUS", where);
+ return false;
+}
+
attr->contiguous = 1;
return gfc_check_conflict (attr, name, where);
}
diff --git a/gcc/testsuite/gfortran.dg/contiguous_12.f90 b/gcc/testsuite/gfortran.dg/contiguous_12.f90
new file mode 100644
index 000..9c477a7a06a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/contiguous_12.f90
@@ -0,0 +1,7 @@
+! { dg-do compile }
+! PR fortran/108025
+
+subroutine foo (x)
+ real, contiguous :: x(:)
+ contiguous :: x! { dg-error "Duplicate CONTIGUOUS attribute" }
+end
--
2.35.3
Re: [PATCH] Fortran: diagnose and reject duplicate CONTIGUOUS attribute [PR108025]
On Thu, Dec 08, 2022 at 10:59:42PM +0100, Harald Anlauf via Fortran wrote: > Dear all, > > a fairly obvious, or rather trivial fix that appeared while > analyzing another pr and that can be treated independently: > reject duplicate CONTIGUOUS attributes. > > (Intel and NAG reject this, Cray warns that this is non-standard.) > > Regtested on x86_64-pc-linux-gnu. OK for mainline? Yes, thanks for the patch. -- Steve
