From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by sourceware.org (Postfix) with ESMTPS id CD45E3858C35 for ; Sun, 7 Apr 2024 06:26:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD45E3858C35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CD45E3858C35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.223.131 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712471181; cv=none; b=q3rKHBRmNjg8DVwd2sgZgEGAekauOszfBNQA0RVmsNQ2TFvGIfJOX60H9J0EcFw5yx2J9kqbyPq01KQZbkM7NZbqNDe4FgJhzd1QI2jsKhlQ7FQFV958fBfI7EbNKh0/gunlHvKw3iwtcXAJVxnCm+Jdu9NI2JYSA1LnmP/RviE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712471181; c=relaxed/simple; bh=mUbHKMhQYYJ5FdvBzr+I1K8oPcwA7w7atXZacT4+/Yk=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:From: Mime-Version:Subject:Date:Message-Id:To; b=d5aiRJkiceZgMXO9H7uVZ3BXIRrhaJQEgycp/rLuj6FyaDACEniGfvvCZjYedCMIgzoWoYx8xtiHUXuwV8yaZEkpJRZZE+tofWtCMMXP0VdT71q04wF/zJg+IQhM/H1Y22h6p4Ib5vZPJB4377CmipSgrAQgDTGCVFXcgVGMJUk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap2.dmz-prg2.suse.org (imap2.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:98]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id E11E81F84C; Sun, 7 Apr 2024 06:26:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1712471177; h=from:from:reply-to: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=zIaFtftNlsXvYBERc6HX9qT6Ez6YzWk8XotVidntuV8=; b=BOQJafZorFDOUogrWsY/dl3VLfplX0eBayrFRXcq3hvraSDa87TTPp+FaP2d70E1gNHADJ uqlevch/Engn+XBtsPrsqel23cWTmkBijsmkZz9o9VJqRIh7ggwFbBusYNcT7PVrHgWMAr XPJpb1K35lAfvVfL9yoAkCQwsCEgYck= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1712471177; h=from:from:reply-to: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=zIaFtftNlsXvYBERc6HX9qT6Ez6YzWk8XotVidntuV8=; b=ylhd3n7ON9f2GaVPXSPrhwxw3v4C4uLZXXIzgaAt75yaT1Tich/sDLukWhExN4hufSf0mb YZadU7m4EM/OwxBA== Authentication-Results: smtp-out2.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=LNtX452V; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=fYITY5fX DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1712471175; h=from:from:reply-to: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=zIaFtftNlsXvYBERc6HX9qT6Ez6YzWk8XotVidntuV8=; b=LNtX452VrWoBg1ERmr2dBqSsDV5zm0HgyXyTbsOv31B2sRpPIOTgiZ0pR2dITi8ovSxpah yAm5Jy634s26Ds6q3sRGtg+eVZ+9OSHLh1195wk70NyFuMYLp9KhsUyIxnoS8YjJpes7Iz gDi7Mox/BBjF3AkKfPxkg60ErmYRENk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1712471175; h=from:from:reply-to: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=zIaFtftNlsXvYBERc6HX9qT6Ez6YzWk8XotVidntuV8=; b=fYITY5fXbIjsKLcJ6MPH7npTeLu108tJNbLdpKfp5/TmTvyjhfR/c2mCRsBuZdH9Eyel1U 4nIyJMpeq3IIazBg== Received: from imap2.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap2.dmz-prg2.suse.org (Postfix) with ESMTPS id D095813586; Sun, 7 Apr 2024 06:26:15 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap2.dmz-prg2.suse.org with ESMTPSA id gNeoMoc8Emb+JQAAn2gu4w (envelope-from ); Sun, 07 Apr 2024 06:26:15 +0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable From: Richard Biener Mime-Version: 1.0 (1.0) Subject: Re: [PATCH 0/2] Condition coverage fixes Date: Sun, 7 Apr 2024 08:26:05 +0200 Message-Id: References: Cc: gcc-patches@gcc.gnu.org, hjl.tools@gmail.com, hubicka@ucw.cz, ro@cebitec.uni-bielefeld.de In-Reply-To: To: =?utf-8?Q?J=C3=B8rgen_Kvalsvik?= X-Mailer: iPhone Mail (21E236) X-Spam-Score: -3.01 X-Rspamd-Action: no action X-Rspamd-Queue-Id: E11E81F84C X-Spam-Level: X-Rspamd-Server: rspamd2.dmz-prg2.suse.org X-Spamd-Result: default: False [-3.01 / 50.00]; BAYES_HAM(-3.00)[100.00%]; SUSPICIOUS_RECIPS(1.50)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain]; MX_GOOD(-0.01)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FREEMAIL_ENVRCPT(0.00)[gmail.com]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; RBL_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:104:10:150:64:98:from]; FUZZY_BLOCKED(0.00)[rspamd.com]; MIME_TRACE(0.00)[0:+]; ARC_NA(0.00)[]; FREEMAIL_CC(0.00)[gcc.gnu.org,gmail.com,ucw.cz,cebitec.uni-bielefeld.de]; DKIM_TRACE(0.00)[suse.de:+]; TO_DN_SOME(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; MID_RHS_MATCH_FROM(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; RCVD_TLS_ALL(0.00)[]; DWL_DNSWL_BLOCKED(0.00)[suse.de:dkim]; TAGGED_RCPT(0.00)[]; RECEIVED_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:106:10:150:64:167:received]; APPLE_IOS_MAILER_COMMON(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; RCPT_COUNT_FIVE(0.00)[5] X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,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: > Am 06.04.2024 um 22:41 schrieb J=C3=B8rgen Kvalsvik : >=20 > =EF=BB=BFOn 06/04/2024 13:15, J=C3=B8rgen Kvalsvik wrote: >>> On 06/04/2024 07:50, Richard Biener wrote: >>>=20 >>>=20 >>>> Am 05.04.2024 um 21:59 schrieb J=C3=B8rgen Kvalsvik : >>>>=20 >>>> =EF=BB=BFHi, >>>>=20 >>>> I propose these fixes for the current issues with the condition >>>> coverage. >>>>=20 >>>> Rainer, I propose to simply delete the test with __sigsetjmp. I don't >>>> think it actually detects anything reasonable any more, I kept it aroun= d >>>> to prevent a regression. Since then I have built a lot of programs (wit= h >>>> optimization enabled) and not really seen this problem. >>>>=20 >>>> H.J., the problem you found with -O2 was really a problem of >>>> tree-inlining, which was actually caught earlier by Jan [1]. It probabl= y >>>> warrants some more testing, but I could reproduce by tuning your test >>>> case to use always_inline and not -O2 and trigger the error. >>>>=20 >>>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-April/648785.html >>>=20 >>> Ok >> Thanks, committed. >> I am wondering if the fn->cond_uids access should always be guarded (in t= ree-profile.cc) should always be guarded. Right now there is the assumption t= hat if condition coverage is requested the will exist and be populated, but a= s 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 t= he map is not constructed properly? >> Thanks, >> J=C3=B8rgen >=20 > I gave this some more thought, and realised I was too eager to fix the seg= fault. While trunk no longer crashes (at least on my x86_64 linux) the fix i= tself is bad. It copies the gcond -> uid mappings into the caller, but the s= tmts 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. >=20 > I did a very quick prototype to confirm. By applying this patch: >=20 > @@ -2049,6 +2049,9 @@ copy_bb (copy_body_data *id, basic_block bb, >=20 > copy_gsi =3D gsi_start_bb (copy_basic_block); >=20 > + if (!cfun->cond_uids && id->src_cfun->cond_uids) > + cfun->cond_uids =3D new hash_map (); > + > for (gsi =3D 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; >=20 > + if (id->src_cfun->cond_uids && is_a (orig_stmt)) > + { > + unsigned *v =3D id->src_cfun->cond_uids->get (as_a (= orig_stmt)); > + if (v) cfun->cond_uids->put (as_a (stmt), *v); > + } > + >=20 >=20 > and this test program: >=20 > __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; > } >=20 > int > mcdc027e (int a, int b) > { > int y =3D inlinefn (a); > return y + b; > } >=20 >=20 > gcov reports: >=20 > 2: 18:mcdc027e (int a, int b) > condition outcomes covered 1/2 > condition 0 not covered (true) > -: 19:{ > 2: 20: int y =3D inlinefn (a); > 2: 21: return y + b; > -: 22:} >=20 > but without the patch, gcov prints nothing. >=20 > I am not sure if this approach is even ideal. Probably the most problemati= c is the source line mapping which is all messed up. I checked with gcov --b= ranch-probabilities and it too reports the callee at the top of the caller. >=20 > 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 so= urce mapping is a bit surprising. >=20 > What do you think? I am open to other strategies, too I think the most important bit is that the segfault is gone. The interactio= n of coverage with inlining or even other optimization when applying optimiz= ation to coverage should be documented better. Does condition coverage apply ontop of regular coverage counting or is it an= either/or? Thanks, Richard=20 > Thanks, > J=C3=B8rgen >=20 >>>=20 >>> Thanks, >>> Richard >>>=20 >>>=20 >>>> Thanks, >>>> J=C3=B8rgen >>>>=20 >>>> J=C3=B8rgen Kvalsvik (2): >>>> Remove unecessary and broken MC/DC compile test >>>> Copy condition->expr map when inlining [PR114599] >>>>=20 >>>> 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 >>>>=20 >>>> -- >>>> 2.30.2 >>>>=20 >=20