From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.115]) by sourceware.org (Postfix) with ESMTPS id 87B12385DC1F; Thu, 4 Jan 2024 02:18:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 87B12385DC1F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=intel.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 87B12385DC1F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=192.55.52.115 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704334694; cv=none; b=vHhzRpupD3qWirsrS4aUL+6muVbYcA4GzYwUpegwtnk49jsmw+GOpv7sA7ELa+as3D5G5f4IxsqI0sKCmglhf7sEg82ys7PPpyO1AtuSAYdfHHJO6BGbjJASUv1N6lJWe8vkthVgYtZxcvL7CPba8zOmAEwzZKizzOCNynggAJw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704334694; c=relaxed/simple; bh=52vJsiWLUY/sZ6jWEr8bLOiuaLmVp080XySLIbKZsss=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=G73lkeIMd5PzFvOjwkhfKiGy9SFXTMaPEIPEwCP7yZEqjdCi0tG/8PMmgg1KkKsFLNvJy6XfLsH0vNg8FM2YhtDWCMfxTEqwq8tjrE+pND69tmf1BbiLwZDvtEbApC2/2R31AA7q4wtmdlUlT+O41tZToYJPRp0QmeXos56QEFY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1704334691; x=1735870691; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=52vJsiWLUY/sZ6jWEr8bLOiuaLmVp080XySLIbKZsss=; b=T8wphg971iTTt7LELv3S7fRA99TPWw7+g5B/8mOXdreRm+Q8VnKpyojT /wN+zXJMV6MQ5WKRgvxhHi+bH9sTvRbzf6qkOMOMCQtGMWFGczLpQ3zHS 4/JoOaC6EJuHNHliQpUm/IcPWlGV7QcwFvXBwoy0rBkOUaION3ksbbhFh 4/GeecqzCckq796F0xshD5Tl7uAOeRtv3fM1t/u2RvsZ6i0syrGnaeaUS mYsnBo3p+xrT7uK843sIGsy6vbSfmdCtA2Di4nDhIXltuoof7rGkYLfUz sfa8nMG4LAavSdmBa3yEz93LdXC9jkGRYDjsmiQuwVVGgg5Vs3Pm5TfJs g==; X-IronPort-AV: E=McAfee;i="6600,9927,10942"; a="396830072" X-IronPort-AV: E=Sophos;i="6.04,329,1695711600"; d="scan'208";a="396830072" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jan 2024 18:18:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10942"; a="729977076" X-IronPort-AV: E=Sophos;i="6.04,329,1695711600"; d="scan'208";a="729977076" Received: from zhulipeng-win.ccr.corp.intel.com (HELO [10.238.1.222]) ([10.238.1.222]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jan 2024 18:18:05 -0800 Message-ID: Date: Thu, 4 Jan 2024 10:18:03 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD Content-Language: en-US To: Tobias Burnus , fortran@gcc.gnu.org, gcc-patches@gcc.gnu.org Cc: rep.dot.nop@gmail.com, tkoenig@netcologne.de, jakub@redhat.com, Richard.Earnshaw@arm.com, thomas@codesourcery.com, hongjiu.lu@intel.com, tianyou.li@intel.com, pan.deng@intel.com, wangyang.guo@intel.com References: <20231222023605.3894839-1-lipeng.zhu@intel.com> From: Lipeng Zhu Organization: Intel In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00,BODY_8BITS,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 > > 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 >