Re: [PATCH] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD

[Lipeng Zhu <lipeng.zhu@intel.com>]

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
>