From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id C1ABC3861031 for ; Thu, 25 Feb 2021 16:39:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C1ABC3861031 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-592-uD4pH6q6Nr6cs-KJF-jQCA-1; Thu, 25 Feb 2021 11:39:05 -0500 X-MC-Unique: uD4pH6q6Nr6cs-KJF-jQCA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BF84E1966322; Thu, 25 Feb 2021 16:39:04 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-112-197.ams2.redhat.com [10.36.112.197]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 59C7A5D9D2; Thu, 25 Feb 2021 16:39:03 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 11PGd1S51621997 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 25 Feb 2021 17:39:01 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 11PGcxVx1621996; Thu, 25 Feb 2021 17:38:59 +0100 Date: Thu, 25 Feb 2021 17:38:59 +0100 From: Jakub Jelinek To: Kwok Cheung Yeung Cc: GCC Patches Subject: Re: [WIP] Re: [PATCH] openmp: Fix intermittent hanging of task-detach-6 libgomp tests [PR98738] Message-ID: <20210225163859.GI4020736@tucnak> Reply-To: Jakub Jelinek References: <20210129150317.GN4020736@tucnak> <0aca6daf-356a-7b03-c007-e33c35114356@codesourcery.com> <20210222134943.GA1837485@tucnak> <253a1682-5e95-c8a1-4274-ad1d9db65c44@codesourcery.com> <20210224194625.GC4020736@tucnak> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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: Thu, 25 Feb 2021 16:39:09 -0000 On Thu, Feb 25, 2021 at 04:21:31PM +0000, Kwok Cheung Yeung wrote: > Reversing the order reduces the hole to 3 bytes: > > size_t num_dependees; /* 80 8 */ > union { > gomp_sem_t * completion_sem; /* 88 8 */ > struct gomp_team * detach_team; /* 88 8 */ > }; /* 88 8 */ > _Bool deferred_p; /* 96 1 */ > > /* XXX 3 bytes hole, try to pack */ > > int priority; /* 100 4 */ > > If we were really determined to get rid of the hole, we could stash > deferred_p in the least-significant bit of the pointer union, but I think Sorry, indeed, I was thinking about how it would be packed after priority, not before it but putting it in between priority and the related prio queues is undesirable. So union, deferred_p, priority is the right order. > > I think you can move task->deferred_p into the if stmt. > > That can be done (since the code for detach is currently the only thing > using it), but I think it would be better to have deferred_p always have the > right value, regardless of whether or not it is used? Otherwise that might > lead to some confusion if it is later used by something else. Ok either way. > I will get this committed later if the regression tests finish with no surprises. Two more nits below. > @@ -86,7 +87,8 @@ gomp_init_task (struct gomp_task *task, struct gomp_task *parent_task, > task->dependers = NULL; > task->depend_hash = NULL; > task->depend_count = 0; > - task->detach = false; > + task->deferred_p = true; > + task->detach_team = NULL; > } Please initialize deferred_p to false rather than true, because gomp_init_task is called in many places and except for that one spot in GOMP_task (and one in taskloop.c) the tasks are undeferred (e.g. the implicit tasks in parallel or the initial one etc.). And maybe also reorder the fields initialized in there to match the order of increasing field offsets. > @@ -414,16 +411,18 @@ GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *), > task.final_task = (thr->task && thr->task->final_task) > || (flags & GOMP_TASK_FLAG_FINAL); > task.priority = priority; > + task.deferred_p = false; And then this shouldn't be here, gomp_init_task has already initialized it that way. Jakub