On Mon, 29 Aug 2022, Martin Jambor wrote: > Hi again, > > On Mon, Aug 29 2022, Richard Biener wrote: > > On Fri, 26 Aug 2022, Martin Jambor wrote: > > > >> Hi, > >> > >> On Fri, Aug 26 2022, Richard Biener wrote: > >> >> Am 26.08.2022 um 18:39 schrieb Martin Jambor <mjam...@suse.cz>: > >> >> > >> >> Hi, > >> >> > >> >> This patch adds constructors of array_slice that are required to > >> >> create them from non-const (heap or auto) vectors or from GC vectors. > >> >> > >> >> The use of non-const array_slices is somewhat limited, as creating one > >> >> from const vec<some_type> still leads to array_slice<const some_type>, > >> >> so I eventually also only resorted to having read-only array_slices. > >> >> But I do need the constructor from the gc vector. > >> >> > >> >> Bootstrapped and tested along code that actually uses it on > >> >> x86_64-linux. OK for trunk? > >> >> > >> >> Thanks, > >> >> > >> >> Martin > >> >> > >> >> > >> >> gcc/ChangeLog: > >> >> > >> >> 2022-08-08 Martin Jambor <mjam...@suse.cz> > >> >> > >> >> * vec.h (array_slice): Add constructors for non-const reference to > >> >> heap vector and pointers to heap vectors. > >> >> --- > >> >> gcc/vec.h | 12 ++++++++++++ > >> >> 1 file changed, 12 insertions(+) > >> >> > >> >> diff --git a/gcc/vec.h b/gcc/vec.h > >> >> index eed075addc9..b0477e1044c 100644 > >> >> --- a/gcc/vec.h > >> >> +++ b/gcc/vec.h > >> >> @@ -2264,6 +2264,18 @@ public: > >> >> array_slice (const vec<OtherT> &v) > >> >> : m_base (v.address ()), m_size (v.length ()) {} > >> >> > >> >> + template<typename OtherT> > >> >> + array_slice (vec<OtherT> &v) > >> >> + : m_base (v.address ()), m_size (v.length ()) {} > >> >> + > >> >> + template<typename OtherT> > >> >> + array_slice (const vec<OtherT, va_gc> *v) > >> >> + : m_base (v ? v->address () : nullptr), m_size (v ? v->length () : > >> >> 0) {} > >> >> + > >> >> + template<typename OtherT> > >> >> + array_slice (vec<OtherT, va_gc> *v) > >> >> + : m_base (v ? v->address () : nullptr), m_size (v ? v->length () : > >> >> 0) {} > >> >> + > >> > > >> > I don?t quite understand why the generic ctor doesn?t cover the GC case. > >> > It looks more like reference vs pointer? > >> > > >> > >> If you think that this should work: > >> > >> vec<tree, va_gc> *heh = cfun->local_decls; > >> array_slice<tree> arr_slice (*heh); > >> > >> then it does not: > >> > >> /home/mjambor/gcc/mine/src/gcc/ipa-cp.cc:6693:36: error: no matching > >> function for call to ?array_slice<tree_node*>::array_slice(vec<tree_node*, > >> va_gc>&)? > >> 6693 | array_slice<tree> arr_slice (*heh); > >> | ^ > >> In file included from /home/mjambor/gcc/mine/src/gcc/hash-table.h:248, > >> from /home/mjambor/gcc/mine/src/gcc/coretypes.h:486, > >> from /home/mjambor/gcc/mine/src/gcc/ipa-cp.cc:105: > >> /home/mjambor/gcc/mine/src/gcc/vec.h:2264:3: note: candidate: > >> ?template<class OtherT> array_slice<T>::array_slice(const vec<OtherT>&) > >> [with T = tree_node*]? > >> 2264 | array_slice (const vec<OtherT> &v) > >> | ^~~~~~~~~~~ > >> /home/mjambor/gcc/mine/src/gcc/vec.h:2264:3: note: template argument > >> deduction/substitution failed: > >> /home/mjambor/gcc/mine/src/gcc/ipa-cp.cc:6693:36: note: mismatched > >> types ?va_heap? and ?va_gc? > >> 6693 | array_slice<tree> arr_slice (*heh); > >> | ^ > >> > >> [... I trimmed notes about all other candidates...] > >> > >> Or did you mean something else? > > > > Hmm, so what if you change > > > > template<typename OtherT> > > array_slice (const vec<OtherT> &v) > > : m_base (v.address ()), m_size (v.length ()) {} > > > > to > > > > template<typename OtherT, typename l, typename a> > > array_slice (const vec<OtherT, l, a> &v) > > : m_base (v.address ()), m_size (v.length ()) {} > > > > instead? Thus allow any allocation / placement template arg? > > > > So being fully awake helps, the issue was of course in how I tested the > code, the above works fine and I can adapt my code to use that. > > However, is it really preferable? > > We often use NULL as to mean zero-length vector, which my code handled > gracefully: > > + template<typename OtherT> > + array_slice (const vec<OtherT, va_gc> *v) > + : m_base (v ? v->address () : nullptr), m_size (v ? v->length () : 0) {} > > whereas using the generic method will mean that users constructing the > vector will have to special case it - and I bet most will end up using > the above sequence and the constructor from explicit pointer and size in > all constructors from gc vectors. > > So, should I really change the patch and my code?
Well, it's also inconsistent with a supposed use like vec<tree> *v = NULL; auto slice = array_slice (v); no? So, if we want to provide a "safe" (as in, handle NULL pointer) CTOR, don't we want to handle non-GC allocated vectors the same way? Btw, we have template<size_t N> array_slice (T (&array)[N]) : m_base (array), m_size (N) {} which would suggest handling NULL isn't desired(?) Richard.