From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id 111FE3858004 for ; Tue, 19 Jan 2021 08:46:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 111FE3858004 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ChungLin_Tang@mentor.com IronPort-SDR: ZkVuANw/x/HAAYrnym9O/CdLZO0N6Lltqmysn+EBdd/4fKhoGpoVmkxu7IBkRxkjqtZAL4FzaL qRoouJE1AbJs3I03X4z+Q2Qv8WrNL7BOnNNVzVZ46ZAL9rHEfz4JKHdFRxvqnf5VZUIvR9k+dH ra8mBjofOH4Ut6FywOxI1esdzkNFVgSJqxnia1q1+pcY4Mp57uPs/CLJb+fUU2aswQpn9Lfp4m XbFig4YDWsoA702q+pvu5enAGXeWW5vN1i6lBRBXO4cJJWdFWXNOohjMICaTEQwGX+BiMjyJJe Hoc= X-IronPort-AV: E=Sophos;i="5.79,358,1602576000"; d="scan'208";a="57192145" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa2.mentor.iphmx.com with ESMTP; 19 Jan 2021 00:46:45 -0800 IronPort-SDR: WwbafV1lYXgrl+7kIfZFDvxcdM3hQe8jxkR8B+neoAZrOk/t4h+/fT0PS5vLUUq7daQmiLMMlh DAxCSFpoL/elclJq3lBVYGIHNpDl0b57bhqKDyAbLvm9LTkaN51GEZrFfKqLSb0OEjSO8H2Xsk AijY9yvkuP3QaLyno/kIj3DmXeWq8OvzP2aBFKSfkku8Upl68Yrtgxv7jjg8M6VsGb/SZX/gpX BHC+3XwZByS0FXbm05tNqX9EEZli7MSJPSS6hn1gjnM8FZn2tQTq7WL0dNvZT77X+zMQgFlbsv ANI= Subject: Re: [PATCH, v2, OpenMP 5.0, libgomp] Structure element mapping for OpenMP 5.0 To: Jakub Jelinek CC: gcc-patches , Catherine Moore , Andrew Stubbs References: <12b667d2-09fe-0640-2622-c78ab0b52f87@codesourcery.com> <20210116094518.GA1034503@tucnak> From: Chung-Lin Tang Message-ID: Date: Tue, 19 Jan 2021 16:46:36 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <20210116094518.GA1034503@tucnak> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-ClientProxiedBy: SVR-ORW-MBX-07.mgc.mentorg.com (147.34.90.207) To svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jan 2021 08:46:48 -0000 On 2021/1/16 5:45 下午, Jakub Jelinek wrote: >> + /* Unified reference count for structure element siblings, this is used >> + when REFCOUNT_STRUCTELEM_FIRST_P(k->refcount) == true, the first sibling >> + in a structure element sibling list item sequence. */ >> + uintptr_t structelem_refcount; >> + >> + /* When REFCOUNT_STRUCTELEM_P (k->refcount) == true, this field points > > REFCOUNT_STRUCTELEM_P (k->refcount) is true even for > REFCOUNT_STRUCTELEM_FIRST_P(k->refcount), so shouldn't the description say > that structelem_refcount_ptr is only used if > REFCOUNT_STRUCTELEM_P (k->refcount) && !REFCOUNT_STRUCTELEM_FIRST_P (k->refcount) > ? Sure, I'll revise the comments a bit. >> + into the (above) structelem_refcount field of the _FIRST splay_tree_key, >> + the first key in the created sequence. All structure element siblings >> + share a single refcount in this manner. Since these two fields won't be >> + used at the same time, they are stashed in a union. */ >> + uintptr_t *structelem_refcount_ptr; >> + }; >> struct splay_tree_aux *aux; >> }; >> >> /* The comparison function. */ > > Anyway, most of the patch looks good, but I'd like to understand the > rationale for choosing a htab over what I've been trying to suggest, which > was essentially instead of incrementing or decrementing refcounts push them > into a vector for later incrementing/decrementing, then qsort the vector > (by the pointers to refcounts) and increment what the elements point to unless > the same address has been incremented/decremented already. > > Jakub Essentially the requirement is to increment/decrement a refcount only once per construct, so using a pointer-set (implemented by htab_t here) to track the processing status seemed to be more intuitive in code, and probably faster than sorting a vector I think (at least in most cases). Chung-Lin