> > 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<type_pair> *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; }