From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx.kolabnow.com (mx.kolabnow.com [212.103.80.155]) by sourceware.org (Postfix) with ESMTPS id 3D1213858D34 for ; Sat, 6 Apr 2024 20:41:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3D1213858D34 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=lambda.is Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=lambda.is ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3D1213858D34 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=212.103.80.155 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712436085; cv=none; b=AzLOTcNmrx1DHVuOfuBnMug/w3GtYAfSULTzQ5Cqi/mMoHX70wOex7WBtVrSL3nJl/CnVHNAqC97PumzfHpHg2vlZJSZTXOuUQEkOs0p1UjicSmxassu56VTcZCwd13iTJ3J0pMYW4cBOZFbfJbmj7LUp7kxLal4FA5Vof0AieE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712436085; c=relaxed/simple; bh=wPmKQea6cE0JB4qBG1FQuc9TkTWeeX7OwvDU3XmcIGE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=LajrM2wRvoWxJKuLBf3XTriwrjWQasO798+ZrfWCsDqZCeay8Z7EEDTexP9cc0d/SO+JdGJHb/LY/rbD9AtWYzKEZtjixXzZ+uAju6CNFg5CggLOHYq10WO/RCIuRJDaCF25Vx7WLYDCn+c6oxwZ4vPoCm/nOCeywy+6ZWm1MFs= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from localhost (unknown [127.0.0.1]) by mx.kolabnow.com (Postfix) with ESMTP id 9CC612096DC5; Sat, 6 Apr 2024 22:41:21 +0200 (CEST) Authentication-Results: ext-mx-out011.mykolab.com (amavis); dkim=pass (4096-bit key) reason="pass (just generated, assumed good)" header.d=kolabnow.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kolabnow.com; h= content-transfer-encoding:content-type:content-type:in-reply-to :content-language:references:from:from:subject:subject :mime-version:date:date:message-id:received:received:received; s=dkim20160331; t=1712436078; x=1714250479; bh=vri9Ahc3jdJuIT/T oZgXXqbT18h7R1xcpB6pYCWzcNs=; b=Yyv7qjn8Jw0v68wgByeuD1r5XVt4cpzz VQMQXgmUJ1+RW91v5US/q2JR6cbFYpyekOTOE3lXpm0APZ4zfW4ArDpv7BOWtglM 7a/IBDNcUAGe8HPt8f5RdpaLalGNseg55Rwi3cwFJRMVKY9fk2Wj5dPSyTmxBfDR laQAaIa1Us041CVtn1nMZ+oo8TbTDnhGL91o50wRjYPK7NzzXF+iFgcoyv7KwGhB KBpfHVh5T1H/soBrLlmOrqWNqc9JfF/EXPSzLPOIAM0lMZxI8BRvDhkTJVRxHxA7 Wp0wnEZYncYUeCh61oaIgMY7JAbSwoEZT8c4rhaIJT9TyQ2S2knwzf8SKK90gAm1 VI4nkE8N23HmE9YVADYBkMQG/z1jCDq1kgvuh60xWWY8VL53GnoarmIoXL84KPdA vR7NmITSwY+VFSyxasjKourM5BPu0W1n0poi5IFOxRFyM88CH3qKnMwMVe15b94q y4WeSyC01lNAupu1npNdba1gPNAoevUQA9C+ZwFTN8iJssiokjEVcVX3dg/iUHUf O32Kc2GpLBSaeQR/LBlnQp99J1nSFSxQm6uAPOYoatddCrTfd4xzueO0iJ1ff1Q7 creXzDixF0v6/QPQ20WrMkrKq67V1gdPzcUxblupliCm8bIHLiFj1pEqUmKXjTDp 7sNvl100zA8= X-Virus-Scanned: amavis at mykolab.com X-Spam-Score: -1 X-Spam-Level: X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 Received: from mx.kolabnow.com ([127.0.0.1]) by localhost (ext-mx-out011.mykolab.com [127.0.0.1]) (amavis, port 10024) with ESMTP id SOMPBjAoCkdQ; Sat, 6 Apr 2024 22:41:18 +0200 (CEST) Received: from int-mx011.mykolab.com (unknown [10.9.13.11]) by mx.kolabnow.com (Postfix) with ESMTPS id 479EB2096DC4; Sat, 6 Apr 2024 22:41:16 +0200 (CEST) Received: from ext-subm010.mykolab.com (unknown [10.9.6.10]) by int-mx011.mykolab.com (Postfix) with ESMTPS id 8650530FC154; Sat, 6 Apr 2024 22:41:16 +0200 (CEST) Message-ID: Date: Sat, 6 Apr 2024 22:41:14 +0200 MIME-Version: 1.0 Subject: Re: [PATCH 0/2] Condition coverage fixes From: =?UTF-8?Q?J=C3=B8rgen_Kvalsvik?= To: Richard Biener Cc: gcc-patches@gcc.gnu.org, hjl.tools@gmail.com, hubicka@ucw.cz, ro@cebitec.uni-bielefeld.de References: <20240405195831.2728853-1-j@lambda.is> <26d42e56-5ac8-4a36-bcaf-72a41d6ba447@lambda.is> Content-Language: en-US In-Reply-To: <26d42e56-5ac8-4a36-bcaf-72a41d6ba447@lambda.is> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 06/04/2024 13:15, Jørgen Kvalsvik wrote: > On 06/04/2024 07:50, Richard Biener wrote: >> >> >>> Am 05.04.2024 um 21:59 schrieb Jørgen Kvalsvik : >>> >>> Hi, >>> >>> I propose these fixes for the current issues with the condition >>> coverage. >>> >>> Rainer, I propose to simply delete the test with __sigsetjmp. I don't >>> think it actually detects anything reasonable any more, I kept it around >>> to prevent a regression. Since then I have built a lot of programs (with >>> optimization enabled) and not really seen this problem. >>> >>> H.J., the problem you found with -O2 was really a problem of >>> tree-inlining, which was actually caught earlier by Jan [1]. It probably >>> warrants some more testing, but I could reproduce by tuning your test >>> case to use always_inline and not -O2 and trigger the error. >>> >>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-April/648785.html >> >> Ok > > Thanks, committed. > > I am wondering if the fn->cond_uids access should always be guarded (in > tree-profile.cc) should always be guarded. Right now there is the > assumption that if condition coverage is requested the will exist and be > populated, but as this shows there may be other circumstances where this > is not true. > > Or perhaps there should be a gcc_assert to (reliably) detect cases where > the map is not constructed properly? > > Thanks, > Jørgen I gave this some more thought, and realised I was too eager to fix the segfault. While trunk no longer crashes (at least on my x86_64 linux) the fix itself is bad. It copies the gcond -> uid mappings into the caller, but the stmts are deep copied into the caller, so no gcond will ever be a hit when we look up the condition_uids in tree-profile.cc. I did a very quick prototype to confirm. By applying this patch: @@ -2049,6 +2049,9 @@ copy_bb (copy_body_data *id, basic_block bb, copy_gsi = gsi_start_bb (copy_basic_block); + if (!cfun->cond_uids && id->src_cfun->cond_uids) + cfun->cond_uids = new hash_map (); + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { gimple_seq stmts; @@ -2076,6 +2079,12 @@ copy_bb (copy_body_data *id, basic_block bb, if (gimple_nop_p (stmt)) continue; + if (id->src_cfun->cond_uids && is_a (orig_stmt)) + { + unsigned *v = id->src_cfun->cond_uids->get (as_a (orig_stmt)); + if (v) cfun->cond_uids->put (as_a (stmt), *v); + } + and this test program: __attribute__((always_inline)) inline int inlinefn (int a) { if (a > 5) { printf ("a > 5\n"); return a; } else printf ("a < 5, was %d\n", a); return a * a - 2; } int mcdc027e (int a, int b) { int y = inlinefn (a); return y + b; } gcov reports: 2: 18:mcdc027e (int a, int b) condition outcomes covered 1/2 condition 0 not covered (true) -: 19:{ 2: 20: int y = inlinefn (a); 2: 21: return y + b; -: 22:} but without the patch, gcov prints nothing. I am not sure if this approach is even ideal. Probably the most problematic is the source line mapping which is all messed up. I checked with gcov --branch-probabilities and it too reports the callee at the top of the caller. If you think it is a good strategy I can clean up the prototype and submit a patch. I suppose the function _totals_ should be accurate, even if the source mapping is a bit surprising. What do you think? I am open to other strategies, too Thanks, Jørgen > >> >> Thanks, >> Richard >> >> >>> Thanks, >>> Jørgen >>> >>> Jørgen Kvalsvik (2): >>>   Remove unecessary and broken MC/DC compile test >>>   Copy condition->expr map when inlining [PR114599] >>> >>> gcc/testsuite/gcc.misc-tests/gcov-19.c       | 11 --------- >>> gcc/testsuite/gcc.misc-tests/gcov-pr114599.c | 25 ++++++++++++++++++++ >>> gcc/tree-inline.cc                           | 20 +++++++++++++++- >>> 3 files changed, 44 insertions(+), 12 deletions(-) >>> create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr114599.c >>> >>> -- >>> 2.30.2 >>> >