Re: [PATCH 05/13] allow constructing a auto_vec with a preallocation, and a possibly larger actual allocation size
Trevor Saunderswrites: > On Thu, May 11, 2017 at 10:01:51AM +0200, Richard Biener wrote: >> On Thu, May 11, 2017 at 9:45 AM, Trevor Saunders >> wrote: >> > On Wed, May 10, 2017 at 07:54:13AM +0100, Richard Sandiford wrote: >> >> tbsaunde+...@tbsaunde.org writes: >> >> > From: Trevor Saunders >> >> > >> >> > This allows us to set the capacity of the vector when we construct it, >> >> > and still use a stack buffer when the size is small enough. >> >> > >> >> > gcc/ChangeLog: >> >> > >> >> > 2017-05-09 Trevor Saunders >> >> > >> >> > * genrecog.c (int_set::int_set): Explicitly construct our >> >> > auto_vec base class. >> >> > * vec.h (auto_vec::auto_vec): New constructor. >> >> > --- >> >> > gcc/genrecog.c | 8 +--- >> >> > gcc/vec.h | 12 >> >> > 2 files changed, 17 insertions(+), 3 deletions(-) >> >> > >> >> > diff --git a/gcc/genrecog.c b/gcc/genrecog.c >> >> > index 6a9e610e7a0..b69043f0d02 100644 >> >> > --- a/gcc/genrecog.c >> >> > +++ b/gcc/genrecog.c >> >> > @@ -1407,14 +1407,16 @@ struct int_set : public auto_vec >> >> >iterator end (); >> >> > }; >> >> > >> >> > -int_set::int_set () {} >> >> > +int_set::int_set () : auto_vec () {} >> >> > >> >> > -int_set::int_set (uint64_t label) >> >> > +int_set::int_set (uint64_t label) : >> >> > + auto_vec () >> >> > { >> >> >safe_push (label); >> >> > } >> >> > >> >> > -int_set::int_set (const int_set ) >> >> > +int_set::int_set (const int_set ) : >> >> > + auto_vec () >> >> > { >> >> >safe_splice (other); >> >> > } >> >> >> >> Is this part of the patch necessary? Won't the default constructor >> >> be used anyway? >> > >> > Well, without the change to the copy constructor we get this bootstrap >> > warning. >> > >> > /src/gcc/gcc/genrecog.c: In copy constructor ‘int_set::int_set(const >> > int_set&)’: >> > /src/gcc/gcc/genrecog.c:1417:1: error: base class ‘class auto_vec> > unsigned int, 1>’ should be explicitly initialized in the copy >> > constructor [-Werror=extra] >> > int_set::int_set (const int_set ) >> > ^~~ >> > >> >> >> > So we need to do something about that. I'm not sure the other cases are >> > necessary, but I was there, and being explicit seemed better than >> > leaving it implicit. >> >> Ah, >> >> /* If these initializations are taking place in a copy constructor, >> the base class should probably be explicitly initialized if >> there >> is a user-defined constructor in the base class (other than the >> default constructor, which will be called anyway). */ >> if (extra_warnings >> && DECL_COPY_CONSTRUCTOR_P (current_function_decl) >> && type_has_user_nondefault_constructor (BINFO_TYPE >> (subobject))) >> warning_at (DECL_SOURCE_LOCATION (current_function_decl), >> OPT_Wextra, "base class %q#T should be explicitly " >> "initialized in the copy constructor", >> BINFO_TYPE (subobject)); >> >> ok - fine then. Probably could be avoided with >> >> auto_vec() = defaulted; >> >> (or how you'd write that) > > Well, we don't get to use = default in C++98, so we'd have to ifdef, I > guess it could work since it would fix the warning outside of stage 1, > but seems pretty gross. Yeah. OK for the genrecog.c bit. Thanks, Richard
Re: [PATCH 05/13] allow constructing a auto_vec with a preallocation, and a possibly larger actual allocation size
On Thu, May 11, 2017 at 10:01:51AM +0200, Richard Biener wrote: > On Thu, May 11, 2017 at 9:45 AM, Trevor Saunders> wrote: > > On Wed, May 10, 2017 at 07:54:13AM +0100, Richard Sandiford wrote: > >> tbsaunde+...@tbsaunde.org writes: > >> > From: Trevor Saunders > >> > > >> > This allows us to set the capacity of the vector when we construct it, > >> > and still use a stack buffer when the size is small enough. > >> > > >> > gcc/ChangeLog: > >> > > >> > 2017-05-09 Trevor Saunders > >> > > >> > * genrecog.c (int_set::int_set): Explicitly construct our > >> > auto_vec base class. > >> > * vec.h (auto_vec::auto_vec): New constructor. > >> > --- > >> > gcc/genrecog.c | 8 +--- > >> > gcc/vec.h | 12 > >> > 2 files changed, 17 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/gcc/genrecog.c b/gcc/genrecog.c > >> > index 6a9e610e7a0..b69043f0d02 100644 > >> > --- a/gcc/genrecog.c > >> > +++ b/gcc/genrecog.c > >> > @@ -1407,14 +1407,16 @@ struct int_set : public auto_vec > >> >iterator end (); > >> > }; > >> > > >> > -int_set::int_set () {} > >> > +int_set::int_set () : auto_vec () {} > >> > > >> > -int_set::int_set (uint64_t label) > >> > +int_set::int_set (uint64_t label) : > >> > + auto_vec () > >> > { > >> >safe_push (label); > >> > } > >> > > >> > -int_set::int_set (const int_set ) > >> > +int_set::int_set (const int_set ) : > >> > + auto_vec () > >> > { > >> >safe_splice (other); > >> > } > >> > >> Is this part of the patch necessary? Won't the default constructor > >> be used anyway? > > > > Well, without the change to the copy constructor we get this bootstrap > > warning. > > > > /src/gcc/gcc/genrecog.c: In copy constructor ‘int_set::int_set(const > > int_set&)’: > > /src/gcc/gcc/genrecog.c:1417:1: error: base class ‘class auto_vec > unsigned int, 1>’ should be explicitly initialized in the copy > > constructor [-Werror=extra] > > int_set::int_set (const int_set ) > > ^~~ > > > >> > > So we need to do something about that. I'm not sure the other cases are > > necessary, but I was there, and being explicit seemed better than > > leaving it implicit. > > Ah, > > /* If these initializations are taking place in a copy constructor, > the base class should probably be explicitly initialized if there > is a user-defined constructor in the base class (other than the > default constructor, which will be called anyway). */ > if (extra_warnings > && DECL_COPY_CONSTRUCTOR_P (current_function_decl) > && type_has_user_nondefault_constructor (BINFO_TYPE > (subobject))) > warning_at (DECL_SOURCE_LOCATION (current_function_decl), > OPT_Wextra, "base class %q#T should be explicitly " > "initialized in the copy constructor", > BINFO_TYPE (subobject)); > > ok - fine then. Probably could be avoided with > > auto_vec() = defaulted; > > (or how you'd write that) Well, we don't get to use = default in C++98, so we'd have to ifdef, I guess it could work since it would fix the warning outside of stage 1, but seems pretty gross. Trev > > Thanks, > Richard. > > > Thanks > > > > Trev > > > >> Thanks, > >> Richard > >> > >> > diff --git a/gcc/vec.h b/gcc/vec.h > >> > index fee46164b01..914f89c350c 100644 > >> > --- a/gcc/vec.h > >> > +++ b/gcc/vec.h > >> > @@ -1272,6 +1272,18 @@ public: > >> > this->m_vec = _auto; > >> >} > >> > > >> > + auto_vec (size_t s) > >> > + { > >> > +if (s > N) > >> > + { > >> > + this->create (s); > >> > + return; > >> > + } > >> > + > >> > +m_auto.embedded_init (MAX (N, 2), 0, 1); > >> > +this->m_vec = _auto; > >> > + } > >> > + > >> >~auto_vec () > >> >{ > >> > this->release ();
Re: [PATCH 05/13] allow constructing a auto_vec with a preallocation, and a possibly larger actual allocation size
On Thu, May 11, 2017 at 9:45 AM, Trevor Saunderswrote: > On Wed, May 10, 2017 at 07:54:13AM +0100, Richard Sandiford wrote: >> tbsaunde+...@tbsaunde.org writes: >> > From: Trevor Saunders >> > >> > This allows us to set the capacity of the vector when we construct it, >> > and still use a stack buffer when the size is small enough. >> > >> > gcc/ChangeLog: >> > >> > 2017-05-09 Trevor Saunders >> > >> > * genrecog.c (int_set::int_set): Explicitly construct our >> > auto_vec base class. >> > * vec.h (auto_vec::auto_vec): New constructor. >> > --- >> > gcc/genrecog.c | 8 +--- >> > gcc/vec.h | 12 >> > 2 files changed, 17 insertions(+), 3 deletions(-) >> > >> > diff --git a/gcc/genrecog.c b/gcc/genrecog.c >> > index 6a9e610e7a0..b69043f0d02 100644 >> > --- a/gcc/genrecog.c >> > +++ b/gcc/genrecog.c >> > @@ -1407,14 +1407,16 @@ struct int_set : public auto_vec >> >iterator end (); >> > }; >> > >> > -int_set::int_set () {} >> > +int_set::int_set () : auto_vec () {} >> > >> > -int_set::int_set (uint64_t label) >> > +int_set::int_set (uint64_t label) : >> > + auto_vec () >> > { >> >safe_push (label); >> > } >> > >> > -int_set::int_set (const int_set ) >> > +int_set::int_set (const int_set ) : >> > + auto_vec () >> > { >> >safe_splice (other); >> > } >> >> Is this part of the patch necessary? Won't the default constructor >> be used anyway? > > Well, without the change to the copy constructor we get this bootstrap > warning. > > /src/gcc/gcc/genrecog.c: In copy constructor ‘int_set::int_set(const > int_set&)’: > /src/gcc/gcc/genrecog.c:1417:1: error: base class ‘class auto_vec unsigned int, 1>’ should be explicitly initialized in the copy > constructor [-Werror=extra] > int_set::int_set (const int_set ) > ^~~ > >> > So we need to do something about that. I'm not sure the other cases are > necessary, but I was there, and being explicit seemed better than > leaving it implicit. Ah, /* If these initializations are taking place in a copy constructor, the base class should probably be explicitly initialized if there is a user-defined constructor in the base class (other than the default constructor, which will be called anyway). */ if (extra_warnings && DECL_COPY_CONSTRUCTOR_P (current_function_decl) && type_has_user_nondefault_constructor (BINFO_TYPE (subobject))) warning_at (DECL_SOURCE_LOCATION (current_function_decl), OPT_Wextra, "base class %q#T should be explicitly " "initialized in the copy constructor", BINFO_TYPE (subobject)); ok - fine then. Probably could be avoided with auto_vec() = defaulted; (or how you'd write that) Thanks, Richard. > Thanks > > Trev > >> Thanks, >> Richard >> >> > diff --git a/gcc/vec.h b/gcc/vec.h >> > index fee46164b01..914f89c350c 100644 >> > --- a/gcc/vec.h >> > +++ b/gcc/vec.h >> > @@ -1272,6 +1272,18 @@ public: >> > this->m_vec = _auto; >> >} >> > >> > + auto_vec (size_t s) >> > + { >> > +if (s > N) >> > + { >> > + this->create (s); >> > + return; >> > + } >> > + >> > +m_auto.embedded_init (MAX (N, 2), 0, 1); >> > +this->m_vec = _auto; >> > + } >> > + >> >~auto_vec () >> >{ >> > this->release ();
Re: [PATCH 05/13] allow constructing a auto_vec with a preallocation, and a possibly larger actual allocation size
On Wed, May 10, 2017 at 07:54:13AM +0100, Richard Sandiford wrote: > tbsaunde+...@tbsaunde.org writes: > > From: Trevor Saunders> > > > This allows us to set the capacity of the vector when we construct it, > > and still use a stack buffer when the size is small enough. > > > > gcc/ChangeLog: > > > > 2017-05-09 Trevor Saunders > > > > * genrecog.c (int_set::int_set): Explicitly construct our > > auto_vec base class. > > * vec.h (auto_vec::auto_vec): New constructor. > > --- > > gcc/genrecog.c | 8 +--- > > gcc/vec.h | 12 > > 2 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/gcc/genrecog.c b/gcc/genrecog.c > > index 6a9e610e7a0..b69043f0d02 100644 > > --- a/gcc/genrecog.c > > +++ b/gcc/genrecog.c > > @@ -1407,14 +1407,16 @@ struct int_set : public auto_vec > >iterator end (); > > }; > > > > -int_set::int_set () {} > > +int_set::int_set () : auto_vec () {} > > > > -int_set::int_set (uint64_t label) > > +int_set::int_set (uint64_t label) : > > + auto_vec () > > { > >safe_push (label); > > } > > > > -int_set::int_set (const int_set ) > > +int_set::int_set (const int_set ) : > > + auto_vec () > > { > >safe_splice (other); > > } > > Is this part of the patch necessary? Won't the default constructor > be used anyway? Well, without the change to the copy constructor we get this bootstrap warning. /src/gcc/gcc/genrecog.c: In copy constructor ‘int_set::int_set(const int_set&)’: /src/gcc/gcc/genrecog.c:1417:1: error: base class ‘class auto_vec’ should be explicitly initialized in the copy constructor [-Werror=extra] int_set::int_set (const int_set ) ^~~ > So we need to do something about that. I'm not sure the other cases are necessary, but I was there, and being explicit seemed better than leaving it implicit. Thanks Trev > Thanks, > Richard > > > diff --git a/gcc/vec.h b/gcc/vec.h > > index fee46164b01..914f89c350c 100644 > > --- a/gcc/vec.h > > +++ b/gcc/vec.h > > @@ -1272,6 +1272,18 @@ public: > > this->m_vec = _auto; > >} > > > > + auto_vec (size_t s) > > + { > > +if (s > N) > > + { > > + this->create (s); > > + return; > > + } > > + > > +m_auto.embedded_init (MAX (N, 2), 0, 1); > > +this->m_vec = _auto; > > + } > > + > >~auto_vec () > >{ > > this->release ();
Re: [PATCH 05/13] allow constructing a auto_vec with a preallocation, and a possibly larger actual allocation size
tbsaunde+...@tbsaunde.org writes: > From: Trevor Saunders> > This allows us to set the capacity of the vector when we construct it, > and still use a stack buffer when the size is small enough. > > gcc/ChangeLog: > > 2017-05-09 Trevor Saunders > > * genrecog.c (int_set::int_set): Explicitly construct our > auto_vec base class. > * vec.h (auto_vec::auto_vec): New constructor. > --- > gcc/genrecog.c | 8 +--- > gcc/vec.h | 12 > 2 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/gcc/genrecog.c b/gcc/genrecog.c > index 6a9e610e7a0..b69043f0d02 100644 > --- a/gcc/genrecog.c > +++ b/gcc/genrecog.c > @@ -1407,14 +1407,16 @@ struct int_set : public auto_vec >iterator end (); > }; > > -int_set::int_set () {} > +int_set::int_set () : auto_vec () {} > > -int_set::int_set (uint64_t label) > +int_set::int_set (uint64_t label) : > + auto_vec () > { >safe_push (label); > } > > -int_set::int_set (const int_set ) > +int_set::int_set (const int_set ) : > + auto_vec () > { >safe_splice (other); > } Is this part of the patch necessary? Won't the default constructor be used anyway? Thanks, Richard > diff --git a/gcc/vec.h b/gcc/vec.h > index fee46164b01..914f89c350c 100644 > --- a/gcc/vec.h > +++ b/gcc/vec.h > @@ -1272,6 +1272,18 @@ public: > this->m_vec = _auto; >} > > + auto_vec (size_t s) > + { > +if (s > N) > + { > + this->create (s); > + return; > + } > + > +m_auto.embedded_init (MAX (N, 2), 0, 1); > +this->m_vec = _auto; > + } > + >~auto_vec () >{ > this->release ();
[PATCH 05/13] allow constructing a auto_vec with a preallocation, and a possibly larger actual allocation size
From: Trevor SaundersThis allows us to set the capacity of the vector when we construct it, and still use a stack buffer when the size is small enough. gcc/ChangeLog: 2017-05-09 Trevor Saunders * genrecog.c (int_set::int_set): Explicitly construct our auto_vec base class. * vec.h (auto_vec::auto_vec): New constructor. --- gcc/genrecog.c | 8 +--- gcc/vec.h | 12 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/gcc/genrecog.c b/gcc/genrecog.c index 6a9e610e7a0..b69043f0d02 100644 --- a/gcc/genrecog.c +++ b/gcc/genrecog.c @@ -1407,14 +1407,16 @@ struct int_set : public auto_vec iterator end (); }; -int_set::int_set () {} +int_set::int_set () : auto_vec () {} -int_set::int_set (uint64_t label) +int_set::int_set (uint64_t label) : + auto_vec () { safe_push (label); } -int_set::int_set (const int_set ) +int_set::int_set (const int_set ) : + auto_vec () { safe_splice (other); } diff --git a/gcc/vec.h b/gcc/vec.h index fee46164b01..914f89c350c 100644 --- a/gcc/vec.h +++ b/gcc/vec.h @@ -1272,6 +1272,18 @@ public: this->m_vec = _auto; } + auto_vec (size_t s) + { +if (s > N) + { + this->create (s); + return; + } + +m_auto.embedded_init (MAX (N, 2), 0, 1); +this->m_vec = _auto; + } + ~auto_vec () { this->release (); -- 2.11.0