> > > I don't know what the guidance is on using vec in IPA passes > > > but with respect to existing practice elsewhere, there are > > > existing uses of vec and auto_vec with non-POD types and vec > > > does work with them (see the vec_default_construct and > > > vec_copy_construct templates, for example, whose goal is > > > to support nontrivial classes). > > > > I see, since 2017 :). The patch is OK then. > > Nontrivial destructors also behave in a sane way these days? > > Good question :) > > At a minimum, element dtors should be automatically invoked by > the auto_vec dtor (there is an auto-test in vec.c to verify that). > > Beyond that, since (unlike auto_vec) a plain vec isn't a container > its users are on their own when it comes to managing the memory of > their elements (i.e., they need to explicitly destroy their elements). > > Having said that, as with all retrofits, they could be incomplete. > I see a few examples of where that seems to be the case here: > > Calling truncate() on a vec with notrivial elements leaks, so > clients needs to explicitly release those elements. That > should happen automatically. > > Going through vec, I also see calls to memmove in functions > like quick_insert, ordered_remove, and block_remove. So calling > those functions is not safe on a vec with nontrivial types. > > Calling any of the sort functions also may not work correctly > with nontrivial elements (gcc_sort() calls memcpy). vec should > either prevent that buy refusing to compile or use a safe > (generic) template for that. > > So while basic vec uses work with nontrivial types, there are > plenty of bugs :(
OK, sounds bit dangerous but the use here is very simple. I remember the non-POD rule mostly from the original David's implementation of modref that did put non-pods to vector and he took really long while to work out why it breaks. And yep, it was before 2017 :) Honza > > Martin > > > > > Honza > > > > > > Martin > > > > > > > The diagnostics should be from > > > > a.parm_offset_known &= m.parm_offset_known; > > > > Becasue both in the parm_map (which is variable m) and access_node > > > > (which is variable a) the parm_offset_known has no meaning when > > > > parm_index == MODREF_UNKNOWN_PARM. > > > > > > > > If we want to avoid computing on these, perhaps this will work? > > > > > > > > diff --git a/gcc/ipa-modref-tree.h b/gcc/ipa-modref-tree.h > > > > index 0a097349ebd..97736d0d8a4 100644 > > > > --- a/gcc/ipa-modref-tree.h > > > > +++ b/gcc/ipa-modref-tree.h > > > > @@ -568,9 +568,13 @@ struct GTY((user)) modref_tree > > > > : (*parm_map) [a.parm_index]; > > > > if (m.parm_index == > > > > MODREF_LOCAL_MEMORY_PARM) > > > > continue; > > > > - a.parm_offset += m.parm_offset; > > > > - a.parm_offset_known &= m.parm_offset_known; > > > > a.parm_index = m.parm_index; > > > > + if (a.parm_index != MODREF_UNKNOWN_PARM) > > > > + { > > > > + a.parm_offset_known &= > > > > m.parm_offset_known; > > > > + if (a.parm_offset_known) > > > > + a.parm_offset += m.parm_offset; > > > > + } > > > > } > > > > } > > > > changed |= insert (base_node->base, ref_node->ref, > > > > a, > > > > > /* Index of parameter we translate to. > > > > > Values from special_params enum are permitted too. */ > > > > > int parm_index; > > > > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > > > > > index c94f0589d44..630d202d5cf 100644 > > > > > --- a/gcc/ipa-modref.c > > > > > +++ b/gcc/ipa-modref.c > > > > > @@ -5020,8 +5020,7 @@ ipa_merge_modref_summary_after_inlining > > > > > (cgraph_edge *edge) > > > > > auto_vec <modref_parm_map, 32> parm_map; > > > > > modref_parm_map chain_map; > > > > > /* TODO: Once we get jump functions for static chains we > > > > > could > > > > > - compute this. */ > > > > > - chain_map.parm_index = MODREF_UNKNOWN_PARM; > > > > > + compute parm_index. */ > > > > > compute_parm_map (edge, &parm_map); > > > > > -- > > > > > 2.33.1 > > > > > > > > >