On Mon, Aug 10, 2020 at 02:57:31PM +0200, Aldy Hernandez wrote:
> > I think it would be nice to see e.g. in -fdump-tree-gimple dump
> > on ipa-fnsummary.c what value_range ctors does it call for the auto_vec and
> > if that is what we want, other than that LGTM.
>
> I see the following in the dump:
>
> evaluate_properties_for_edge()
> {
> ...
> struct auto_vec known_value_ranges;
> ...
> try
> {
> ...
> auto_vec<int_range<1>, 32>::auto_vec (&known_value_ranges);
> ...
>
> It looks like the auto_vec constructor is calling the int_range<1>
> constructor correctly:
>
> auto_vec<int_range<1>, 32>::auto_vec (struct auto_vec * const this)
> {
> ...
> int_range<1>::int_range (D.130911);
>
>
> The int_range<1> constructor also looks correct:
>
> int_range<1>::int_range (struct int_range * const this, const struct
> int_range & other)
> {
> *this = {CLOBBER};
> {
> _1 = &this->D.92184;
> _2 = &this->m_ranges;
> irange::irange (_1, _2, 1);
> _3 = &this->D.92184;
> _4 = &other->D.92184;
> irange::operator= (_3, _4);
> }
> }
>
> If y'all agree, I will push the patch with Jakub's suggested changes.
I believe even with that change vec doesn't work properly with (arbitrary)
non-PODs that need non-trivial construction or destruction.
I know you haven't introduced that yourself, but just would like to discuss
the semantics we want.
To test, I've tried:
--- vec.c.xx 2020-01-12 11:54:38.533381424 +0100
+++ vec.c 2020-08-10 15:33:40.600680476 +0200
@@ -544,9 +544,34 @@ test_auto_delete_vec ()
/* Run all of the selftests within this file. */
+struct SSXS
+{
+ SSXS () { __builtin_printf ("SSXS ()\n"); }
+ SSXS (const SSXS &) { __builtin_printf ("SSXS (const SSXS &)\n"); }
+ SSXS &operator= (const SSXS &) { __builtin_printf ("operator= (const SSXS
&)\n"); }
+ ~SSXS () { __builtin_printf ("~SSXS ()\n"); }
+ int s;
+};
+
void
vec_c_tests ()
{
+ {
+ auto_vec<SSXS, 8> ssxs;
+ __builtin_printf ("constructed\n");
+ SSXS ssxso;
+ __builtin_printf ("ssxso\n");
+ for (int i = 0; i < 16; i++)
+ {
+ __builtin_printf ("safe_push start\n");
+ {
+ ssxs.safe_push (ssxso);
+ }
+ __builtin_printf ("safe_push end\n");
+ }
+ __builtin_printf ("about to destruct\n");
+ }
+ __builtin_printf ("destructed\n");
test_quick_push ();
test_safe_push ();
test_truncate ();
and I see following. Let me add comments to that.
#So auto_vec constructor will default initialize all m_data
#elements (i.e. all the embedded objects):
SSXS ()
SSXS ()
SSXS ()
SSXS ()
SSXS ()
SSXS ()
SSXS ()
SSXS ()
constructed
#And now the extra object I've created.
SSXS ()
ssxso
# Now, 8 times we invoke an assignment operator, the objects
# are already constructed, so all is good.
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
# Now, 9th safe_push, we ran out of room in the embedded buffer.
# vec_copy_construct will now copy construct the 8 elements
# in the new array which will be used from now on.
safe_push start
SSXS (const SSXS &)
SSXS (const SSXS &)
SSXS (const SSXS &)
SSXS (const SSXS &)
SSXS (const SSXS &)
SSXS (const SSXS &)
SSXS (const SSXS &)
SSXS (const SSXS &)
# But important note, we do not construct the object at new m_vecdata[8],
# only call assignment operator on it. That is invalid C++, isn't it?
operator= (const SSXS &)
safe_push end
safe_push start
# And again, and again ...
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
safe_push start
operator= (const SSXS &)
safe_push end
about to destruct
# Now we destruct the 8 embedded elts in auto_vec plus the variable
# I had there, but the 16 elts of the m_vecdata array, 8 of those
# were copy constructed and then operator= modified, while
# the other 8 were only operator= set, are not destructed ever.
~SSXS ()
~SSXS ()
~SSXS ()
~SSXS ()
~SSXS ()
~SSXS ()
~SSXS ()
~SSXS ()
~SSXS ()
destructed
So, for vec on types with non-trivial construction and/or destruction, the
question is, do we want to construct the elements whenever underlying
storage for those is allocated (i.e. as now let the embedded buffer have
constructed elements and whenever we allocate new memory for the vector,
next to the vec_copy_construct where we copy construct the oldsize elts,
we also default construct the remaining ones) and then destruct when
deallocating the memory and then just use assignment operator to push stuff
in, or do we want the memory uninitialized instead and only copy construct
during quick_push and destruct during pop etc., i.e. only the currently
active elts are constructed and nothing else? E.g. auto_vec m_data would
then need to be a different type (e.g. char array with appropriate alignas
and size) which wouldn't need non-trivial construction and destruction.
For types with trivial construction/destruction, I'd like to keep current
behavior...
Jakub
