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 107653858C50 for ; Thu, 9 Feb 2023 07:44:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 107653858C50 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=suse.cz 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 35B7A34E35; Thu, 9 Feb 2023 07:44:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1675928646; 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=Ty2HNbxNavI3Dr2Fx1zsat0Tz+vAVuwh78CnGj8op+s=; b=Eec/3I9yIzh1LGzAxBWBfUkOhwvc+9y2C0f5Iw6qyr/IS0E9v2Af1lIcCN2d/ZZp04lsDV EsyU27FZEGzAnFOPGNrjKTjjkFcNNxBIge7w5ncLj4AjErScK5Z7rC23gH7/hnh26fqmGE iJsfgADLhO77dM2Hv2xksn29AhYcmHM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1675928646; 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=Ty2HNbxNavI3Dr2Fx1zsat0Tz+vAVuwh78CnGj8op+s=; b=qZlt1OE8kq0OFJjkznY1To8ETj0zXWHRq34lKn5ZpHqlCPLcwxhdSrPekB5ClJUn1oKARD uNTCFg4O6uEwnTCA== 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 1E15313A1F; Thu, 9 Feb 2023 07:44:06 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id HLhYBkak5GNMAQAAMHmgww (envelope-from ); Thu, 09 Feb 2023 07:44:06 +0000 Message-ID: <08e947d9-2d38-974e-2a84-35a931919e75@suse.cz> Date: Thu, 9 Feb 2023 08:44:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] ipa: silent -Wodr notes with -w To: Jan Hubicka Cc: Martin Jambor , gcc-patches@gcc.gnu.org References: <160b53bd-0747-9699-2fef-62415bb6450f@suse.cz> Content-Language: en-US From: =?UTF-8?Q?Martin_Li=c5=a1ka?= In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.1 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_SOFTFAIL,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 2/9/23 01:10, Jan Hubicka wrote: >>> On 2/1/23 15:26, Martin Jambor wrote: >>>> Hi, >>>> >>>> On Fri, Dec 02 2022, Martin Liška wrote: >>>>> If -w is used, warn_odr properly sets *warned = false and >>>>> so it should be preserved when calling warn_types_mismatch. >>>>> >>>>> Noticed that during a LTO reduction where I used -w. >>>>> >>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >>>>> >>>>> Ready to be installed? >>>>> Thanks, >>>>> Martin >>>>> >>>>> gcc/ChangeLog: >>>>> >>>>> * ipa-devirt.cc (odr_types_equivalent_p): Respect *warned >>>>> value if set. >>>> >>> >>> Hi. >>> >>>> Sorry for skipping this for so long, usually ODR stuff is... interesting >>>> to the point I gladly leave it to Honza. >>> >>> Makes sense, however, he's not much active when it comes to patch review. >> >> Sorry, I was confused by the patch and delayed reply to figure out what >> you are trying to fix. Indeed the dererence is missing here, however >> every caller that sets warn to true should also set warned to non-NULL. >> So indeed derefernce is missing, but I think the check for >> warned == NULL should not be necessary. > > This seems to bootstrap with LTO. I am not sure what testcase you > looked in, but unless there as a good reason to include the NULL check, > I would rmeove it as it makes it harder to see what is going on. Hi. Thanks for the patch. Apparently, I noticed that during reduction of a test-case where I used -w in order to silent all warnings. So go ahead with your patch. Martin > > honza > > diff --git a/gcc/ipa-devirt.cc b/gcc/ipa-devirt.cc > index 14cf132c767..819860258d1 100644 > --- a/gcc/ipa-devirt.cc > +++ b/gcc/ipa-devirt.cc > @@ -1221,6 +1221,9 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned, > hash_set *visited, > location_t loc1, location_t loc2) > { > + /* If we are asked to warn, we need warned to keep track if warning was > + output. */ > + gcc_assert (!warn || warned); > /* Check first for the obvious case of pointer identity. */ > if (t1 == t2) > return true; > @@ -1300,7 +1303,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned, > warn_odr (t1, t2, NULL, NULL, warn, warned, > G_("it is defined as a pointer to different type " > "in another translation unit")); > - if (warn && (warned == NULL || *warned)) > + if (warn && *warned) > warn_types_mismatch (TREE_TYPE (t1), TREE_TYPE (t2), > loc1, loc2); > return false; > @@ -1315,7 +1318,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned, > warn_odr (t1, t2, NULL, NULL, warn, warned, > G_("a different type is defined " > "in another translation unit")); > - if (warn && (warned == NULL || *warned)) > + if (warn && *warned) > warn_types_mismatch (TREE_TYPE (t1), TREE_TYPE (t2), loc1, loc2); > return false; > } > @@ -1333,7 +1336,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned, > warn_odr (t1, t2, NULL, NULL, warn, warned, > G_("a different type is defined in another " > "translation unit")); > - if (warn && (warned == NULL || *warned)) > + if (warn && *warned) > warn_types_mismatch (TREE_TYPE (t1), TREE_TYPE (t2), loc1, loc2); > } > gcc_assert (TYPE_STRING_FLAG (t1) == TYPE_STRING_FLAG (t2)); > @@ -1375,7 +1378,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned, > warn_odr (t1, t2, NULL, NULL, warn, warned, > G_("has different return value " > "in another translation unit")); > - if (warn && (warned == NULL || *warned)) > + if (warn && *warned) > warn_types_mismatch (TREE_TYPE (t1), TREE_TYPE (t2), loc1, loc2); > return false; > } > @@ -1398,7 +1401,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned, > warn_odr (t1, t2, NULL, NULL, warn, warned, > G_("has different parameters in another " > "translation unit")); > - if (warn && (warned == NULL || *warned)) > + if (warn && *warned) > warn_types_mismatch (TREE_VALUE (parms1), > TREE_VALUE (parms2), loc1, loc2); > return false; > @@ -1484,7 +1487,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned, > warn_odr (t1, t2, f1, f2, warn, warned, > G_("a field of same name but different type " > "is defined in another translation unit")); > - if (warn && (warned == NULL || *warned)) > + if (warn && *warned) > warn_types_mismatch (TREE_TYPE (f1), TREE_TYPE (f2), loc1, loc2); > return false; > }