Re: [PATCH v4] libgfortran: Replace mutex with rwlock
[CCing Ian as libgcc maintainer] On Wed, 1 Nov 2023 10:14:37 + "Zhu, Lipeng" wrote: > > > > > > Hi Lipeng, > > > > > > >>> Sure, as your comments, in the patch V6, I added 3 test cases with > > > >>> OpenMP to test different cases in concurrency respectively: > > > >>> 1. find and create unit very frequently to stress read lock and write > > > >>> lock. > > > >>> 2. only access the unit which exist in cache to stress read lock. > > > >>> 3. access the same unit in concurrency. > > > >>> For the third test case, it also help to find a bug: When unit > > > >>> can't be found in cache nor unit list in read phase, then threads > > > >>> will try to acquire write lock to insert the same unit, this will > > > >>> cause duplicate key > > > >> error. > > > >>> To fix this bug, I get the unit from unit list once again before > > > >>> insert in write > > > >> lock. > > > >>> More details you can refer the patch v6. > > > >>> > > > >> > > > >> Could you help to review this update? I really appreciate your > > > >> assistance. > > > >> > > > > > > > Could you help to review this update? Any concern will be appreciated. > > > > > > > > > > Fortran parts are OK (I think I wrote that already), we need somebody > > > for the non-Fortran parts. > > > > > Hi Thomas, > > > > Thanks for your response. Very appreciate for your patience and help. > > > > > Jakub, could you maybe take a look? > > > > > > Best regards > > > > > > Thomas > > > > Hi Jakub, > > > > Can you help to take a look at the change for libgcc part that added several > > rwlock macros in libgcc/gthr-posix.h? > > > > Hi Jakub, > > Could you help to review this, any comment will be greatly appreciated. Latest version is at https://inbox.sourceware.org/gcc-patches/20230818031818.2161842-1-lipeng@intel.com/ > > > Best Regards, > > Lipeng Zhu >
Re: [Patch, fortran] PR98498 - Interp request: defined operators and unlimited polymorphic
Hi Harald,
I was overthinking the problem. The rejected cases led me to a fix that can
only be described as a considerable simplification compared with the first
patch!
The testcase now reflects the requirements of the standard and
regtests without failures.
OK for mainline?
Thanks
Paul
Fortran: Defined operators with unlimited polymorphic args [PR98498]
2023-11-02 Paul Thomas
gcc/fortran
PR fortran/98498
* interface.cc (upoly_ok): Defined operators using unlimited
polymorphic formal arguments must not override the intrinsic
operator use.
gcc/testsuite/
PR fortran/98498
* gfortran.dg/interface_50.f90: New test.
On Wed, 1 Nov 2023 at 20:12, Harald Anlauf wrote:
> Hi Paul,
>
> Am 01.11.23 um 19:02 schrieb Paul Richard Thomas:
> > The interpretation request came in a long time ago but I only just got
> > around to implementing it.
> >
> > The updated text from the standard is in the comment. Now I am writing
> > this, I think that I should perhaps use switch(op)/case rather than using
> > if/else if and depending on the order of the gfc_intrinsic_op enum being
> > maintained. Thoughts?
>
> the logic is likely harder to parse with if/else than with
> switch(op)/case. However, I do not think that the order of
> the enum will ever be changed, as the module format relies
> on that very order.
>
> > The testcase runs fine with both mainline and nagfor. I think that
> > compile-only with counts of star-eq and star_not should suffice.
>
> I found other cases that are rejected even with your patch,
> but which are accepted by nagfor. Example:
>
> print *, ('a' == c)
>
> Nagfor prints F at runtime as expected, as it correctly resolves
> this to star_eq. Further examples can be easily constructed.
>
> Can you have a look?
>
> Thanks,
> Harald
>
> > Regtests with no regressions. OK for mainline?
> >
> > Paul
> >
> > Fortran: Defined operators with unlimited polymorphic args [PR98498]
> >
> > 2023-11-01 Paul Thomas
> >
> > gcc/fortran
> > PR fortran/98498
> > * interface.cc (upoly_ok): New function.
> > (gfc_extend_expr): Use new function to ensure that defined
> > operators using unlimited polymorphic formal arguments do not
> > override their intrinsic uses.
> >
> > gcc/testsuite/
> > PR fortran/98498
> > * gfortran.dg/interface_50.f90: New test.
> >
>
>
diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index 8c4571e0aa6..fc4fe662eab 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -4737,6 +4737,17 @@ gfc_extend_expr (gfc_expr *e)
if (sym != NULL)
break;
}
+
+ /* F2018(15.4.3.4.2) requires that the use of unlimited polymorphic
+ formal arguments does not override the intrinsic uses. */
+ gfc_push_suppress_errors ();
+ if (sym
+ && (UNLIMITED_POLY (sym->formal->sym)
+ || (sym->formal->next
+ && UNLIMITED_POLY (sym->formal->next->sym)))
+ && !gfc_check_operator_interface (sym, e->value.op.op, e->where))
+ sym = NULL;
+ gfc_pop_suppress_errors ();
}
/* TODO: Do an ambiguity-check and error if multiple matching interfaces are
! { dg-do compile }
! { dg-options "-fdump-tree-original" }
!
! Tests the fix for PR98498, which was subject to an interpretation request
! as to whether or not the interface operator overrode the intrinsic use.
! (See PR for correspondence)
!
! Contributed by Paul Thomas
!
MODULE mytypes
IMPLICIT none
TYPE pvar
character(len=20) :: name
integer :: level
end TYPE pvar
interface operator (==)
module procedure star_eq
end interface
interface operator (.not.)
module procedure star_not
end interface
contains
function star_eq(a, b)
implicit none
class(*), intent(in) :: a, b
logical :: star_eq
select type (a)
type is (pvar)
select type (b)
type is (pvar)
if((a%level .eq. b%level) .and. (a%name .eq. b%name)) then
star_eq = .true.
else
star_eq = .false.
end if
type is (integer)
star_eq = (a%level == b)
end select
class default
star_eq = .false.
end select
end function star_eq
function star_not (a)
implicit none
class(*), intent(in) :: a
type(pvar) :: star_not
select type (a)
type is (pvar)
star_not = a
star_not%level = -star_not%level
type is (real)
star_not = pvar ("real", -int(a))
class default
star_not = pvar ("noname", 0)
end select
end function
end MODULE mytypes
program test_eq
use mytypes
implicit none
type(pvar) x, y
integer :: i = 4
real :: r = 2.0
character(len = 4, kind =4) :: c = "abcd"
! Check that intrinsic use of .not. and == is not overridden.
if (.not.(i == 2*int (r))) stop 1
if (r == 1.0) stop 2
! Test defined operator ==
x = pvar('test 1', 100)
y = pvar('test 1', 100)
if (.not.(x == y)) stop 3
y = pvar('test 2', 100)
if (x == y) stop 4
if (x == r) stop 5! cla
Re: [Patch, fortran] PR98498 - Interp request: defined operators and unlimited polymorphic
Hi Paul,
Am 02.11.23 um 19:18 schrieb Paul Richard Thomas:
Hi Harald,
I was overthinking the problem. The rejected cases led me to a fix that can
only be described as a considerable simplification compared with the first
patch!
this patch is *much* simpler, makes more sense, and works here. :-)
The testcase now reflects the requirements of the standard and
regtests without failures.
OK for mainline?
Yes, OK for mainline.
Thanks,
Harald
Thanks
Paul
Fortran: Defined operators with unlimited polymorphic args [PR98498]
2023-11-02 Paul Thomas
gcc/fortran
PR fortran/98498
* interface.cc (upoly_ok): Defined operators using unlimited
polymorphic formal arguments must not override the intrinsic
operator use.
gcc/testsuite/
PR fortran/98498
* gfortran.dg/interface_50.f90: New test.
On Wed, 1 Nov 2023 at 20:12, Harald Anlauf wrote:
Hi Paul,
Am 01.11.23 um 19:02 schrieb Paul Richard Thomas:
The interpretation request came in a long time ago but I only just got
around to implementing it.
The updated text from the standard is in the comment. Now I am writing
this, I think that I should perhaps use switch(op)/case rather than using
if/else if and depending on the order of the gfc_intrinsic_op enum being
maintained. Thoughts?
the logic is likely harder to parse with if/else than with
switch(op)/case. However, I do not think that the order of
the enum will ever be changed, as the module format relies
on that very order.
The testcase runs fine with both mainline and nagfor. I think that
compile-only with counts of star-eq and star_not should suffice.
I found other cases that are rejected even with your patch,
but which are accepted by nagfor. Example:
print *, ('a' == c)
Nagfor prints F at runtime as expected, as it correctly resolves
this to star_eq. Further examples can be easily constructed.
Can you have a look?
Thanks,
Harald
Regtests with no regressions. OK for mainline?
Paul
Fortran: Defined operators with unlimited polymorphic args [PR98498]
2023-11-01 Paul Thomas
gcc/fortran
PR fortran/98498
* interface.cc (upoly_ok): New function.
(gfc_extend_expr): Use new function to ensure that defined
operators using unlimited polymorphic formal arguments do not
override their intrinsic uses.
gcc/testsuite/
PR fortran/98498
* gfortran.dg/interface_50.f90: New test.
[Patch, fortran] PR112316 - [13 Regression] Fix for PR87477 rejects valid code with a bogus error...
Hi All,
I have pushed as 'obvious' a fix for this regression to both 13-branch and
mainline. The patch itself looks substantial but it consists entirely of
the removal of a condition and repagination of the corresponding block.
Please see below for part of my first comment on the PR for an explanation.
Paul
A temporary work around is to invert the order of the contained procedures.
The problem is caused by a stupid (on my part :-( ) oversight:
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index e103ebee557..f88f9be3be8 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -5196,7 +5196,7 @@ parse_associate (void)
}
}
- if (target->rank)
+ if (1)
{
int rank = 0;
rank = target->rank;
fixes the problem and regtests OK.
