From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 2395B3858D39 for ; Thu, 9 Feb 2023 00:10:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2395B3858D39 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=ucw.cz Authentication-Results: sourceware.org; spf=none smtp.mailfrom=kam.mff.cuni.cz Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id A097E282947; Thu, 9 Feb 2023 01:10:15 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1; t=1675901415; h=from:from:reply-to:subject:subject: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=z7l4L9nBYEM3VAFpCgeTpds4gQgPUgz9Vna01ULUeA8=; b=D8nByBeKc/IWKebIto9Q23QTFGGRXgRp91UeNcTNe2BJXF5BYJ2/8+rrS/ejPk/C7jYCM+ RSbT8ykOBcEsRMuBDHSVXqO6Elub1mPMX6K97cdt+3gPkmei5E0lM7xexhZJcpzwqlbm/G WstjqunaMNaoktaI/TqOPGm5DWl+g6Q= Date: Thu, 9 Feb 2023 01:10:15 +0100 From: Jan Hubicka To: Martin =?iso-8859-2?Q?Li=B9ka?= Cc: Martin Jambor , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] ipa: silent -Wodr notes with -w Message-ID: References: <160b53bd-0747-9699-2fef-62415bb6450f@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-2 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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/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. 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; }