* [PATCH] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD @ 2023-12-22 2:36 Lipeng Zhu 2024-01-03 11:12 ` Tobias Burnus 0 siblings, 1 reply; 7+ messages in thread From: Lipeng Zhu @ 2023-12-22 2:36 UTC (permalink / raw) To: fortran, gcc-patches Cc: rep.dot.nop, tkoenig, jakub, Richard.Earnshaw, thomas, hongjiu.lu, tianyou.li, pan.deng, wangyang.guo, Lipeng Zhu This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is not defined in dec_waiting_unlocked function. libgfortran/ChangeLog: * io/io.h (dec_waiting_unlocked): Use __gthread_rwlock_wrlock/__gthread_rwlock_unlock or __gthread_mutex_lock/__gthread_mutex_unlock functions to replace WRLOCK and RWUNLOCK macros. Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com> --- libgfortran/io/io.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h index 15daa0995b1..c7f0f7d7d9e 100644 --- a/libgfortran/io/io.h +++ b/libgfortran/io/io.h @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) #ifdef HAVE_ATOMIC_FETCH_ADD (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); #else - WRLOCK (&unit_rwlock); +#ifdef __GTHREAD_RWLOCK_INIT + __gthread_rwlock_wrlock (&unit_rwlock); + u->waiting--; + __gthread_rwlock_unlock (&unit_rwlock); +#else + __gthread_mutex_lock (&unit_rwlock); u->waiting--; - RWUNLOCK (&unit_rwlock); + __gthread_mutex_unlock (&unit_rwlock); +#endif #endif } -- 2.39.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD 2023-12-22 2:36 [PATCH] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD Lipeng Zhu @ 2024-01-03 11:12 ` Tobias Burnus 2024-01-04 2:18 ` Lipeng Zhu 0 siblings, 1 reply; 7+ messages in thread From: Tobias Burnus @ 2024-01-03 11:12 UTC (permalink / raw) To: Lipeng Zhu, fortran, gcc-patches Cc: rep.dot.nop, tkoenig, jakub, Richard.Earnshaw, thomas, hongjiu.lu, tianyou.li, pan.deng, wangyang.guo On 22.12.23 03:36, Lipeng Zhu wrote: > This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is > not defined in dec_waiting_unlocked function. > > libgfortran/ChangeLog: > > * io/io.h (dec_waiting_unlocked): Use > __gthread_rwlock_wrlock/__gthread_rwlock_unlock or > __gthread_mutex_lock/__gthread_mutex_unlock functions > to replace WRLOCK and RWUNLOCK macros. > > Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com> The change looks good to me + I assume it will work, but have not tested it myself. Downside is that it slightly breaks with the abstraction done with all the macros, but it seems to be the simplest solution. What is really missing - and should be included in the commit message (before the ChangeLog block) - is the following information: As io.h does not include async.h, the WRLOCK and RWUNLOCK macros are undefined. (Or something similar in other words.) I think that helps others when looking at "git log" and wondering *why* that change was needed. Thanks, Tobias > libgfortran/io/io.h | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h > index 15daa0995b1..c7f0f7d7d9e 100644 > --- a/libgfortran/io/io.h > +++ b/libgfortran/io/io.h > @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) > #ifdef HAVE_ATOMIC_FETCH_ADD > (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); > #else > - WRLOCK (&unit_rwlock); > +#ifdef __GTHREAD_RWLOCK_INIT > + __gthread_rwlock_wrlock (&unit_rwlock); > + u->waiting--; > + __gthread_rwlock_unlock (&unit_rwlock); > +#else > + __gthread_mutex_lock (&unit_rwlock); > u->waiting--; > - RWUNLOCK (&unit_rwlock); > + __gthread_mutex_unlock (&unit_rwlock); > +#endif > #endif > } ----------------- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD 2024-01-03 11:12 ` Tobias Burnus @ 2024-01-04 2:18 ` Lipeng Zhu 2024-01-05 1:43 ` [PATCH v2] " Lipeng Zhu 0 siblings, 1 reply; 7+ messages in thread From: Lipeng Zhu @ 2024-01-04 2:18 UTC (permalink / raw) To: Tobias Burnus, fortran, gcc-patches Cc: rep.dot.nop, tkoenig, jakub, Richard.Earnshaw, thomas, hongjiu.lu, tianyou.li, pan.deng, wangyang.guo On 2024/1/3 19:12, Tobias Burnus wrote: > On 22.12.23 03:36, Lipeng Zhu wrote: >> This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is >> not defined in dec_waiting_unlocked function. >> >> libgfortran/ChangeLog: >> >> * io/io.h (dec_waiting_unlocked): Use >> __gthread_rwlock_wrlock/__gthread_rwlock_unlock or >> __gthread_mutex_lock/__gthread_mutex_unlock functions >> to replace WRLOCK and RWUNLOCK macros. >> >> Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com> > > The change looks good to me + I assume it will work, but have not tested > it myself. > > Downside is that it slightly breaks with the abstraction done with all > the macros, but it seems to be the simplest solution. > Hi Tobias, Thanks for your review, the reason I changed like this is because I found when LOCK macro was first introduced in this patch: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=2b4c90656132abb8b8ad155d345c7d4fbf1687c9, it replaced __gthread_mutex_lock method with LOCK macro in other files like io/unit.c, but remained __gthread_mutex_lock in io/io.h. I am not sure if this is intentional or not, to avoid potential risk, I used the most straightforward solution. > What is really missing - and should be included in the commit message > (before the ChangeLog block) - is the following information: > > As io.h does not include async.h, the WRLOCK and RWUNLOCK macros are > undefined. > > (Or something similar in other words.) > > I think that helps others when looking at "git log" and wondering *why* > that change was needed. > > Thanks, > > Tobias > Thanks, I will update the commit message as suggested. Lipeng Zhu >> libgfortran/io/io.h | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h >> index 15daa0995b1..c7f0f7d7d9e 100644 >> --- a/libgfortran/io/io.h >> +++ b/libgfortran/io/io.h >> @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) >> #ifdef HAVE_ATOMIC_FETCH_ADD >> (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); >> #else >> - WRLOCK (&unit_rwlock); >> +#ifdef __GTHREAD_RWLOCK_INIT >> + __gthread_rwlock_wrlock (&unit_rwlock); >> + u->waiting--; >> + __gthread_rwlock_unlock (&unit_rwlock); >> +#else >> + __gthread_mutex_lock (&unit_rwlock); >> u->waiting--; >> - RWUNLOCK (&unit_rwlock); >> + __gthread_mutex_unlock (&unit_rwlock); >> +#endif >> #endif >> } > ----------------- > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD 2024-01-04 2:18 ` Lipeng Zhu @ 2024-01-05 1:43 ` Lipeng Zhu 2024-01-10 11:52 ` Richard Earnshaw 2024-01-11 12:38 ` Jakub Jelinek 0 siblings, 2 replies; 7+ messages in thread From: Lipeng Zhu @ 2024-01-05 1:43 UTC (permalink / raw) To: tobias Cc: Richard.Earnshaw, fortran, gcc-patches, hongjiu.lu, jakub, pan.deng, rep.dot.nop, thomas, tianyou.li, tkoenig, wangyang.guo, Lipeng Zhu This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is not defined in dec_waiting_unlocked function. As io.h does not include async.h, the WRLOCK and RWUNLOCK macros are undefined. libgfortran/ChangeLog: * io/io.h (dec_waiting_unlocked): Use __gthread_rwlock_wrlock/__gthread_rwlock_unlock or __gthread_mutex_lock/__gthread_mutex_unlock functions to replace WRLOCK and RWUNLOCK macros. Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com> --- libgfortran/io/io.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h index 15daa0995b1..c7f0f7d7d9e 100644 --- a/libgfortran/io/io.h +++ b/libgfortran/io/io.h @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) #ifdef HAVE_ATOMIC_FETCH_ADD (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); #else - WRLOCK (&unit_rwlock); +#ifdef __GTHREAD_RWLOCK_INIT + __gthread_rwlock_wrlock (&unit_rwlock); + u->waiting--; + __gthread_rwlock_unlock (&unit_rwlock); +#else + __gthread_mutex_lock (&unit_rwlock); u->waiting--; - RWUNLOCK (&unit_rwlock); + __gthread_mutex_unlock (&unit_rwlock); +#endif #endif } -- 2.39.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD 2024-01-05 1:43 ` [PATCH v2] " Lipeng Zhu @ 2024-01-10 11:52 ` Richard Earnshaw 2024-01-11 3:05 ` Lipeng Zhu 2024-01-11 12:38 ` Jakub Jelinek 1 sibling, 1 reply; 7+ messages in thread From: Richard Earnshaw @ 2024-01-10 11:52 UTC (permalink / raw) To: Lipeng Zhu, tobias Cc: Richard.Earnshaw, fortran, gcc-patches, hongjiu.lu, jakub, pan.deng, rep.dot.nop, thomas, tianyou.li, tkoenig, wangyang.guo On 05/01/2024 01:43, Lipeng Zhu wrote: > This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is > not defined in dec_waiting_unlocked function. As io.h does > not include async.h, the WRLOCK and RWUNLOCK macros are > undefined. > > libgfortran/ChangeLog: > > * io/io.h (dec_waiting_unlocked): Use > __gthread_rwlock_wrlock/__gthread_rwlock_unlock or > __gthread_mutex_lock/__gthread_mutex_unlock functions > to replace WRLOCK and RWUNLOCK macros. > > Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com> Has this been committed yet? R. > --- > libgfortran/io/io.h | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h > index 15daa0995b1..c7f0f7d7d9e 100644 > --- a/libgfortran/io/io.h > +++ b/libgfortran/io/io.h > @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) > #ifdef HAVE_ATOMIC_FETCH_ADD > (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); > #else > - WRLOCK (&unit_rwlock); > +#ifdef __GTHREAD_RWLOCK_INIT > + __gthread_rwlock_wrlock (&unit_rwlock); > + u->waiting--; > + __gthread_rwlock_unlock (&unit_rwlock); > +#else > + __gthread_mutex_lock (&unit_rwlock); > u->waiting--; > - RWUNLOCK (&unit_rwlock); > + __gthread_mutex_unlock (&unit_rwlock); > +#endif > #endif > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD 2024-01-10 11:52 ` Richard Earnshaw @ 2024-01-11 3:05 ` Lipeng Zhu 0 siblings, 0 replies; 7+ messages in thread From: Lipeng Zhu @ 2024-01-11 3:05 UTC (permalink / raw) To: Richard Earnshaw, tobias Cc: Richard.Earnshaw, fortran, gcc-patches, hongjiu.lu, jakub, pan.deng, rep.dot.nop, thomas, tianyou.li, tkoenig, wangyang.guo On 1/10/2024 7:52 PM, Richard Earnshaw wrote: > On 05/01/2024 01:43, Lipeng Zhu wrote: >> This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is >> not defined in dec_waiting_unlocked function. As io.h does >> not include async.h, the WRLOCK and RWUNLOCK macros are >> undefined. >> >> libgfortran/ChangeLog: >> >> * io/io.h (dec_waiting_unlocked): Use >> __gthread_rwlock_wrlock/__gthread_rwlock_unlock or >> __gthread_mutex_lock/__gthread_mutex_unlock functions >> to replace WRLOCK and RWUNLOCK macros. >> >> Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com> > > Has this been committed yet? > > R. Hi Richard, The patch is waiting for community's review. Hi Tobias, Any concern about this patch? Best Regards, Lipeng Zhu >> --- >> libgfortran/io/io.h | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h >> index 15daa0995b1..c7f0f7d7d9e 100644 >> --- a/libgfortran/io/io.h >> +++ b/libgfortran/io/io.h >> @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) >> #ifdef HAVE_ATOMIC_FETCH_ADD >> (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); >> #else >> - WRLOCK (&unit_rwlock); >> +#ifdef __GTHREAD_RWLOCK_INIT >> + __gthread_rwlock_wrlock (&unit_rwlock); >> + u->waiting--; >> + __gthread_rwlock_unlock (&unit_rwlock); >> +#else >> + __gthread_mutex_lock (&unit_rwlock); >> u->waiting--; >> - RWUNLOCK (&unit_rwlock); >> + __gthread_mutex_unlock (&unit_rwlock); >> +#endif >> #endif >> } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD 2024-01-05 1:43 ` [PATCH v2] " Lipeng Zhu 2024-01-10 11:52 ` Richard Earnshaw @ 2024-01-11 12:38 ` Jakub Jelinek 1 sibling, 0 replies; 7+ messages in thread From: Jakub Jelinek @ 2024-01-11 12:38 UTC (permalink / raw) To: Lipeng Zhu Cc: tobias, Richard.Earnshaw, fortran, gcc-patches, hongjiu.lu, pan.deng, rep.dot.nop, thomas, tianyou.li, tkoenig, wangyang.guo On Thu, Jan 04, 2024 at 08:43:26PM -0500, Lipeng Zhu wrote: > This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is > not defined in dec_waiting_unlocked function. As io.h does > not include async.h, the WRLOCK and RWUNLOCK macros are > undefined. > > libgfortran/ChangeLog: > > * io/io.h (dec_waiting_unlocked): Use > __gthread_rwlock_wrlock/__gthread_rwlock_unlock or > __gthread_mutex_lock/__gthread_mutex_unlock functions > to replace WRLOCK and RWUNLOCK macros. > > Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com> LGTM. Jakub ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-11 12:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-12-22 2:36 [PATCH] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD Lipeng Zhu 2024-01-03 11:12 ` Tobias Burnus 2024-01-04 2:18 ` Lipeng Zhu 2024-01-05 1:43 ` [PATCH v2] " Lipeng Zhu 2024-01-10 11:52 ` Richard Earnshaw 2024-01-11 3:05 ` Lipeng Zhu 2024-01-11 12:38 ` Jakub Jelinek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).