From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125255 invoked by alias); 2 Oct 2019 16:52:59 -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 125246 invoked by uid 89); 2 Oct 2019 16:52:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=HX-Google-Smtp-Source:APXvYqz, overriding X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Oct 2019 16:52:57 +0000 Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9B4B919D335 for ; Wed, 2 Oct 2019 16:52:55 +0000 (UTC) Received: by mail-wr1-f70.google.com with SMTP id f3so1167166wrr.23 for ; Wed, 02 Oct 2019 09:52:55 -0700 (PDT) Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id f143sm14186759wme.40.2019.10.02.09.52.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 02 Oct 2019 09:52:53 -0700 (PDT) Subject: Re: [PATCH 2/2] OpenMP parallel region scope tests To: Kevin Buettner , gdb-patches@sourceware.org References: <20190822171408.28271-1-kevinb@redhat.com> <20190822171408.28271-3-kevinb@redhat.com> From: Pedro Alves Message-ID: Date: Wed, 02 Oct 2019 16:52:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20190822171408.28271-3-kevinb@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-10/txt/msg00069.txt.bz2 Hi, On 8/22/19 6:14 PM, Kevin Buettner 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. > > Also, I have observed runs which show two fewer passes when not > testing against a GDB (plus libgomp w/ plugin) which does not include I guess you meant "when testing" instead of "when not testing"? > my OpenMP work. This is due to the fact that only one stop is made > in the nested_parallel:outer_threads tests. When it happens that the > first (and only) stop occurs for a non-master thread, two KFAILs > result instead of a PASS. > +void > +multi_scope (void) > +{ > + int i01 = 1, i02 = 2; > + > + { > + int i11 = 11, i12 = 12; > + > + { > + int i21 = -21, i22 = 22; > + > +#pragma omp parallel num_threads (2) \ > + firstprivate (i01) \ > + shared (i11) \ > + private (i21) Something odd with indentation here. Tabs vs spaces? > diff --git a/gdb/testsuite/gdb.threads/omp-par-scope.exp b/gdb/testsuite/gdb.threads/omp-par-scope.exp > new file mode 100644 > index 0000000000..719a6123a9 > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/omp-par-scope.exp > @@ -0,0 +1,286 @@ > +# Copyright 2017-2019 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# 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. > + Missing short intro here. > +standard_testfile > + > +if {[gdb_compile_openmp "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != ""} { > + untested "failed to compile OpenMP program" > + return -1 > +} > + > +clean_restart ${binfile} How about adding an "openmp" option to build_executable_from_specs so that you can write: if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {openmp debug}] } { instead of all the above? > + > +# 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 because > +# the output of ldd on the binary may be examined to learn the location > +# of libgomp.so.) > +# > +# 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. 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. Also, maybe rename it to gdb_openmp_setup? As is seems just a little bit too generic. > + > +if {[info procs openmp_setup] != ""} { > + if {[openmp_setup $binfile] != ""} { > + return -1 > + } > +} > + > +if {![runto_main]} { > + untested "could not run to main" > + return -1 > +} > + > +# We want to invoke setup_kfail (and in some cases setup_xfail) when > +# GDB does not yet have support for finding the values of variables in > +# (non-master) threads. We'll check this by looking at the output of > +# "maint print thread-parent". If this command is undefined, then GDB > +# does not yet have thread parent support, and it makes sense to kfail > +# tests which won't work. It's possible for GDB to have this support, > +# but not work. E.g. it may be the case that the plugin doesn't > +# exist or is not found. We may eventually need to add additional > +# constraints related to setting allow_kfail to 0. But, for the moment, > +# this simple test should be sufficient. > + > +set allow_kfail 1 > +gdb_test_multiple "maint print thread-parent" "maint print thread-parent" { > + -re "Undefined maintenance print command.*$gdb_prompt" { > + pass "maint print thread-parent does not exist" All pass/fail calls should have the same message/name. You can add extra info in parens if you want, though, like: pass "maint print thread-parent (does not exist)" > + } > + -re "No parent found.*" { > + pass "maint print thread-parent" > + set allow_kfail 0 Odd indentation. Tabs vs spaces? > + } > + -re ".*$gdb_prompt" { > + fail "maint print thread-parent" > + } This last case is unnecessary, because it's implicit, right? > + > +# 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*] { 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. Also, I think clang does not support nested functions, but it defines __GNUC__. Could you run the test against clang too, please? Thanks, Pedro Alves