From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 606533858D39 for ; Tue, 28 Mar 2023 15:44:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 606533858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 970A221A01; Tue, 28 Mar 2023 15:44:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1680018242; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ie+o5IjKMzu4dladYBCMS1SwUq9FrOlh4poLmWv0cjc=; b=FtzagqF32cmR8kj0NNu3P0bTGRNoH07fnfTJmvk8ssInrAF8V+QNU1Hp/bAVu2tWtI6s6i yVFS5fvbLXc37SLjWNJKgOd6xAv75PnhIhhim4Bi8KSfSFcf+3rgFqAfmnjlVAFvPZp0UF R7m5maciIrqkaRnaPobEzHqokTD187g= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1680018242; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ie+o5IjKMzu4dladYBCMS1SwUq9FrOlh4poLmWv0cjc=; b=Pvvbmx4r7E9bYC2TZxzR4fRiWzNPm7vj+N7xyPkwKrQjz3hOMcCFhKFdgHbSpIk3mjUQL7 ZL75B7Njo2WA/+BQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 840051390B; Tue, 28 Mar 2023 15:44:02 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id m2kbH0ILI2T7IwAAMHmgww (envelope-from ); Tue, 28 Mar 2023 15:44:02 +0000 Message-ID: <0e0254fc-5fa8-71b2-01b8-cc5742909727@suse.de> Date: Tue, 28 Mar 2023 17:44:05 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH] gdb/testsuite: allow "require" callbacks to provide a reason Content-Language: en-US To: Simon Marchi , gdb-patches@sourceware.org References: <20230328122305.6108-1-simon.marchi@efficios.com> From: Tom de Vries In-Reply-To: <20230328122305.6108-1-simon.marchi@efficios.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP 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 3/28/23 14:23, Simon Marchi via Gdb-patches wrote: > When an allow_* proc returns false, it can be a bit difficult what check > failed exactly, if the procedure does multiple checks. To make > investigation easier, I propose to allow the "require" callbacks to be > able to return a list of two elements: the zero/non-zero value, and a > reason string. > Hi, I like the idea. The only question I had was what the desired behaviour is for: ... return {1 "bla"} ... AFAICT, the current implementation just ignores "bla", and I wondered if perhaps we should error out to avoid the impression that something will be done with "bla". That however will make this fail if $res == 1: ... set res [try_foo] return {$res "foo"} ... so probably that's just too restrictive. LGTM as-is. Thanks, - Tom > Use the new feature in allow_hipcc_tests to demonstrate it (it's also > where I hit actually hit this inconvenience). On my computer (where GDB > is built with amd-dbgapi support but where I don't have a suitable GPU > target), I get: > > UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (no suitable amdgpu targets found) > > vs before: > > UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests > > Change-Id: Id1966535b87acfcbe9eac99f49dc1196398c6578 > --- > gdb/testsuite/lib/gdb.exp | 37 +++++++++++++++++++++++++++++-------- > gdb/testsuite/lib/rocm.exp | 8 ++++---- > 2 files changed, 33 insertions(+), 12 deletions(-) > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 46aa1441d6d0..7fb4f1cbdab4 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -9251,22 +9251,43 @@ gdb_caching_proc have_avx {} { > # > # ARG can either be a name, or of the form !NAME. > # > -# Each name is a proc to evaluate in the caller's context. It returns > -# a boolean, and a "!" means to invert the result. If this is > -# nonzero, all is well. If it is zero, an "untested" is emitted and > -# this proc causes the caller to return. > +# Each name is a proc to evaluate in the caller's context. It can return a > +# boolean or a two element list with a boolean and a reason string. > +# A "!" means to invert the result. If this is true, all is well. If it is > +# false, an "unsupported" is emitted and this proc causes the caller to return. > +# > +# The reason string is used to provide some context about a require failure, > +# and is included in the "unsupported" message. > > proc require { args } { > foreach arg $args { > if {[string index $arg 0] == "!"} { > - set ok 0 > + set required_val 0 > set fn [string range $arg 1 end] > } else { > - set ok 1 > + set required_val 1 > set fn $arg > } > - if {$ok != !![uplevel 1 $fn]} { > - unsupported "require failed: $arg" > + > + set result [uplevel 1 $fn] > + set len [llength $result] > + if { $len == 2 } { > + set actual_val [lindex $result 0] > + set msg [lindex $result 1] > + } elseif { $len == 1 } { > + set actual_val $result > + set msg "" > + } else { > + error "proc $fn returned a list of unexpected length $len" > + } > + > + if {$required_val != !!$actual_val} { > + if { [string length $msg] > 0 } { > + unsupported "require failed: $arg ($msg)" > + } else { > + unsupported "require failed: $arg" > + } > + > return -code return 0 > } > } > diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp > index b3e435339dbc..389d73bcaa5f 100644 > --- a/gdb/testsuite/lib/rocm.exp > +++ b/gdb/testsuite/lib/rocm.exp > @@ -64,19 +64,19 @@ gdb_caching_proc allow_hipcc_tests {} { > # testing against GDBserver, there's no point in running the ROCm > # tests. > if {[target_info gdb_protocol] != ""} { > - return 0 > + return {0 "remote debugging"} > } > > # Ensure that GDB is built with amd-dbgapi support. > set output [remote_exec host $::GDB "$::INTERNAL_GDBFLAGS --configuration"] > if { [string first "--with-amd-dbgapi" $output] == -1 } { > - return 0 > + return {0 "amd-dbgapi not supported"} > } > > # Check we have a working hipcc compiler available. > set targets [hcc_amdgpu_targets] > if { [llength $targets] == 0} { > - return 0 > + return {0 "no suitable amdgpu targets found"} > } > > set flags [list hip additional_flags=--offload-arch=[join $targets ","]] > @@ -93,7 +93,7 @@ gdb_caching_proc allow_hipcc_tests {} { > return 0; > } > } executable $flags]} { > - return 0 > + return {0 "failed to compile hip program"} > } > > return 1