From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103028 invoked by alias); 4 Nov 2019 04:49:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 102593 invoked by uid 89); 4 Nov 2019 04:49:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL,BAYES_00,KAM_SHORT autolearn=ham version=3.3.1 spammy=fewer, ended, tabs X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Nov 2019 04:49:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1572842971; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2ikVcyZK/aF1XLdrCtjZ+Os/qBUJRA4brbJNdhabFyk=; b=ZY1KuWUqxGT+Vlv5Tnfrt70AtVd69BWlD/EERFTjtssj7o7hkBodHXULqr/gZVf3tI2Rn1 5CWnOeJ3KH7r64CPLcviiO0SOhCDUeh0+8pA93rhwZuYuUPk4RDWznBlECYnUkT4Zvp4nZ oOUUwPdC0xO1jjyABJADq0dvGnzICKo= 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-23-vaDEbs9tP2aRHDJ_5yfe3g-1; Sun, 03 Nov 2019 23:49:29 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6E45A800A02 for ; Mon, 4 Nov 2019 04:49:28 +0000 (UTC) Received: from f29-4.lan (ovpn-116-78.phx2.redhat.com [10.3.116.78]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4564C1001B35; Mon, 4 Nov 2019 04:49:28 +0000 (UTC) Date: Mon, 04 Nov 2019 04:49:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Cc: Pedro Alves Subject: Re: [PATCH 2/2] OpenMP parallel region scope tests Message-ID: <20191103214927.6d10ac41@f29-4.lan> In-Reply-To: References: <20190822171408.28271-1-kevinb@redhat.com> <20190822171408.28271-3-kevinb@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg00076.txt.bz2 On Wed, 2 Oct 2019 17:52:52 +0100 Pedro Alves wrote: > > Note that the various counts for F27-F30 w/o my work only add up to 150 > > instead of 151. This is due to an extra test (which results in a PASS) > > from running openmp_setup. > >=20 > > Also, I have observed runs which show two fewer passes when not > > testing against a GDB (plus libgomp w/ plugin) which does not include= =20=20 >=20 > I guess you meant "when testing" instead of "when not testing"? I think so, but am no longer certain. I ended up removing that entire paragraph. I think it was getting too far into the weeds. > > +#pragma omp parallel num_threads (2) \ > > + firstprivate (i01) \ > > + shared (i11) \ > > + private (i21)=20=20 >=20 > Something odd with indentation here. Tabs vs spaces? Yes, fixed. > > +# You should have received a copy of the GNU General Public License > > +# along with this program. If not, see . > > + > > +# This file is part of the gdb testsuite. > > +=20=20 >=20 > Missing short intro here. Fixed. > > +standard_testfile > > + > > +if {[gdb_compile_openmp "${srcdir}/${subdir}/${srcfile}" "${binfile}" = executable {debug}] !=3D ""} { > > + untested "failed to compile OpenMP program" > > + return -1 > > +} > > + > > +clean_restart ${binfile}=20=20 >=20 >=20 > How about adding an "openmp" option to build_executable_from_specs > so that you can write: >=20 > if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {o= penmp debug}] } { >=20 > instead of all the above? Done. > > + > > +# openmp_setup may be defined to set auto-load safe-path and possibly > > +# sysroot. These settings are required for gdb to be able to find > > +# the libgomp python plugin. (sysroot only needs to be defined for > > +# remote debugging.) > > +# > > +# This approach has both pros and cons. On the plus side, it's easy > > +# to automatically set a precise auto-load safe-path. (It's easy beca= use > > +# the output of ldd on the binary may be examined to learn the location > > +# of libgomp.so.) > > +#=20 > > +# However, making these settings is also a drawback due to potentially > > +# overriding settings made by a board file. So, for the moment > > +# anyway, this proc is optional and will only be called if it's > > +# defined.=20=20 >=20 > I wonder, should we document this hook somewhere? Maybe > gdb/testsuite/README? Or would you rather keep it more internal > for the moment? I'm fine with that, but I thought I'd ask. For the moment, I'd like to keep it internal. We might go in a different direction with the rest of my work; I don't see much point in documenting something which might later need to be removed. > Also, maybe rename it to gdb_openmp_setup? As is seems just a little > bit too generic. Done. > > +gdb_test_multiple "maint print thread-parent" "maint print thread-pare= nt" { > > + -re "Undefined maintenance print command.*$gdb_prompt" { > > + pass "maint print thread-parent does not exist"=20=20 >=20 > All pass/fail calls should have the same message/name. > You can add extra info in parens if you want, though, like: >=20 > pass "maint print thread-parent (does not exist)" Done. > > + -re "No parent found.*" { > > + pass "maint print thread-parent" > > + set allow_kfail 0=20=20 >=20 > Odd indentation. Tabs vs spaces? Yes, fixed. > > + } > > + -re ".*$gdb_prompt" { > > + fail "maint print thread-parent" > > + }=20=20 >=20 > This last case is unnecessary, because it's implicit, right? Right. I've removed that case. (Fixed.) > > +# Nested functions in C are a GNU extension, so don't do the nest_func > > +# tests unless the compiler is GCC. > > + > > +if [test_compiler_info gcc*] {=20=20 >=20 > Other compilers may implement the extension as well. The Intel compiler > does, I think, for example. I think a better approach is to check some > symbol or value in the program.=20=20 >=20 > Also, I think clang does not support nested functions, but it defines > __GNUC__. Could you run the test against clang too, please? I have now tested against both clang and icc. Testing against clang found some problems in the .c file that have now been fixed. I now have omp-par-scope.exp doing the following: I've wrapped nested_func and its call with... #if HAVE_NESTED_FUNCTION_SUPPORT ... #endif I now have the .exp file attempting to compile the file up to two times, once with -DHAVE_NESTED_FUNCTION_SUPPORT and, if that fails, another time without that -D switch. If no nested function call support exists (as a result of failing that first test), the corresponding tests in the .exp file will not be run. Thanks for your review. I posted an updated patch using gerrit: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/504 Kevin