Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
Richard Biener writes:
> On Mon, 29 Aug 2022, Martin Jambor wrote:
>
>> Hi,
>>
>> On Mon, Aug 29 2022, Richard Biener wrote:
>> > 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 :
>> >> >> >>
>> >> >> >> 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 still leads to array_slice> >> >> >> 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
>> >> >> >>
>> >> >> >>* 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 )
>> >> >> >> : m_base (v.address ()), m_size (v.length ()) {}
>> >> >> >>
>> >> >> >> + template
>> >> >> >> + array_slice (vec )
>> >> >> >> +: m_base (v.address ()), m_size (v.length ()) {}
>> >> >> >> +
>> >> >> >> + template
>> >> >> >> + array_slice (const vec *v)
>> >> >> >> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length
>> >> >> >> () : 0) {}
>> >> >> >> +
>> >> >> >> + template
>> >> >> >> + array_slice (vec *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 *heh = cfun->local_decls;
>> >> >> array_slice 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::array_slice(vec&)?
>> >> >>6693 | array_slice 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 array_slice::array_slice(const
>> >> >> vec&) [with T = tree_node*]?
>> >> >>2264 | array_slice (const vec )
>> >> >> | ^~~
>> >> >> /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 arr_slice (*heh);
>> >> >> |^
>> >> >>
>> >> >> [... I trimmed notes about all other candidates...]
>> >> >>
>> >> >> Or did you mean something else?
>> >> >
>> >> > Hmm, so what if you change
>> >> >
>> >> > template
>> >> > array_slice (const vec )
>> >> > : m_base (v.address ()), m_size (v.length ()) {}
>> >> >
>> >> > to
>> >> >
>> >> > template
>> >> > array_slice (const vec )
>> >> > : 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
>> >> + array_slice (const vec *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
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
On Mon, 29 Aug 2022, Martin Jambor wrote:
> Hi,
>
> On Mon, Aug 29 2022, Richard Biener wrote:
> > 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 :
> >> >> >>
> >> >> >> 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 still leads to array_slice >> >> >> 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
> >> >> >>
> >> >> >>* 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 )
> >> >> >> : m_base (v.address ()), m_size (v.length ()) {}
> >> >> >>
> >> >> >> + template
> >> >> >> + array_slice (vec )
> >> >> >> +: m_base (v.address ()), m_size (v.length ()) {}
> >> >> >> +
> >> >> >> + template
> >> >> >> + array_slice (const vec *v)
> >> >> >> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length
> >> >> >> () : 0) {}
> >> >> >> +
> >> >> >> + template
> >> >> >> + array_slice (vec *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 *heh = cfun->local_decls;
> >> >> array_slice 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::array_slice(vec&)?
> >> >>6693 | array_slice 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 array_slice::array_slice(const vec&)
> >> >> [with T = tree_node*]?
> >> >>2264 | array_slice (const vec )
> >> >> | ^~~
> >> >> /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 arr_slice (*heh);
> >> >> |^
> >> >>
> >> >> [... I trimmed notes about all other candidates...]
> >> >>
> >> >> Or did you mean something else?
> >> >
> >> > Hmm, so what if you change
> >> >
> >> > template
> >> > array_slice (const vec )
> >> > : m_base (v.address ()), m_size (v.length ()) {}
> >> >
> >> > to
> >> >
> >> > template
> >> > array_slice (const vec )
> >> > : 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
> >> + array_slice (const vec *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
>
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
Hi,
On Mon, Aug 29 2022, Richard Biener wrote:
> 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 :
>> >> >>
>> >> >> 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 still leads to array_slice,
>> >> >> 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
>> >> >>
>> >> >>* 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 )
>> >> >> : m_base (v.address ()), m_size (v.length ()) {}
>> >> >>
>> >> >> + template
>> >> >> + array_slice (vec )
>> >> >> +: m_base (v.address ()), m_size (v.length ()) {}
>> >> >> +
>> >> >> + template
>> >> >> + array_slice (const vec *v)
>> >> >> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length ()
>> >> >> : 0) {}
>> >> >> +
>> >> >> + template
>> >> >> + array_slice (vec *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 *heh = cfun->local_decls;
>> >> array_slice 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::array_slice(vec&)?
>> >>6693 | array_slice 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 array_slice::array_slice(const vec&)
>> >> [with T = tree_node*]?
>> >>2264 | array_slice (const vec )
>> >> | ^~~
>> >> /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 arr_slice (*heh);
>> >> |^
>> >>
>> >> [... I trimmed notes about all other candidates...]
>> >>
>> >> Or did you mean something else?
>> >
>> > Hmm, so what if you change
>> >
>> > template
>> > array_slice (const vec )
>> > : m_base (v.address ()), m_size (v.length ()) {}
>> >
>> > to
>> >
>> > template
>> > array_slice (const vec )
>> > : 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
>> + array_slice (const vec *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 *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?
>
Our safe_* functions usually do no work with normal non-GC vectors
(which are not vl_embed), they do not accept them. I guess that is
because that is
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
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 :
> >> >>
> >> >> 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 still leads to array_slice,
> >> >> 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
> >> >>
> >> >>* 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 )
> >> >> : m_base (v.address ()), m_size (v.length ()) {}
> >> >>
> >> >> + template
> >> >> + array_slice (vec )
> >> >> +: m_base (v.address ()), m_size (v.length ()) {}
> >> >> +
> >> >> + template
> >> >> + array_slice (const vec *v)
> >> >> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length () :
> >> >> 0) {}
> >> >> +
> >> >> + template
> >> >> + array_slice (vec *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 *heh = cfun->local_decls;
> >> array_slice 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::array_slice(vec >> va_gc>&)?
> >>6693 | array_slice 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 array_slice::array_slice(const vec&)
> >> [with T = tree_node*]?
> >>2264 | array_slice (const vec )
> >> | ^~~
> >> /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 arr_slice (*heh);
> >> |^
> >>
> >> [... I trimmed notes about all other candidates...]
> >>
> >> Or did you mean something else?
> >
> > Hmm, so what if you change
> >
> > template
> > array_slice (const vec )
> > : m_base (v.address ()), m_size (v.length ()) {}
> >
> > to
> >
> > template
> > array_slice (const vec )
> > : 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
> + array_slice (const vec *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 *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
array_slice (T ()[N]) : m_base (array), m_size (N) {}
which would suggest handling NULL isn't desired(?)
Richard.
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
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 :
>> >>
>> >> 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 still leads to array_slice,
>> >> 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
>> >>
>> >>* 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 )
>> >> : m_base (v.address ()), m_size (v.length ()) {}
>> >>
>> >> + template
>> >> + array_slice (vec )
>> >> +: m_base (v.address ()), m_size (v.length ()) {}
>> >> +
>> >> + template
>> >> + array_slice (const vec *v)
>> >> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length () :
>> >> 0) {}
>> >> +
>> >> + template
>> >> + array_slice (vec *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 *heh = cfun->local_decls;
>> array_slice 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::array_slice(vec> va_gc>&)?
>>6693 | array_slice 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 array_slice::array_slice(const vec&)
>> [with T = tree_node*]?
>>2264 | array_slice (const vec )
>> | ^~~
>> /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 arr_slice (*heh);
>> |^
>>
>> [... I trimmed notes about all other candidates...]
>>
>> Or did you mean something else?
>
> Hmm, so what if you change
>
> template
> array_slice (const vec )
> : m_base (v.address ()), m_size (v.length ()) {}
>
> to
>
> template
> array_slice (const vec )
> : 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
+ array_slice (const vec *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?
Thanks,
Martin
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
Hi,
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 :
>> >>
>> >> 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 still leads to array_slice,
>> >> 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
>> >>
>> >>* 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 )
>> >> : m_base (v.address ()), m_size (v.length ()) {}
>> >>
>> >> + template
>> >> + array_slice (vec )
>> >> +: m_base (v.address ()), m_size (v.length ()) {}
>> >> +
>> >> + template
>> >> + array_slice (const vec *v)
>> >> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length () :
>> >> 0) {}
>> >> +
>> >> + template
>> >> + array_slice (vec *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 *heh = cfun->local_decls;
>> array_slice 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::array_slice(vec> va_gc>&)?
>>6693 | array_slice 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 array_slice::array_slice(const vec&)
>> [with T = tree_node*]?
>>2264 | array_slice (const vec )
>> | ^~~
>> /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 arr_slice (*heh);
>> |^
>>
>> [... I trimmed notes about all other candidates...]
>>
>> Or did you mean something else?
>
> Hmm, so what if you change
>
> template
> array_slice (const vec )
> : m_base (v.address ()), m_size (v.length ()) {}
>
> to
>
> template
> array_slice (const vec )
> : m_base (v.address ()), m_size (v.length ()) {}
>
> instead? Thus allow any allocation / placement template arg?
I tried this on Friday night too (but I was already only half awake) and
it led to some very weird self-test ICEs (and so I went to bed).
(I can try again but debugging such things is not quite what I wanted to
spend my time on :-)
Martin
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
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 :
> >>
> >> 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 still leads to array_slice,
> >> 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
> >>
> >>* 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 )
> >> : m_base (v.address ()), m_size (v.length ()) {}
> >>
> >> + template
> >> + array_slice (vec )
> >> +: m_base (v.address ()), m_size (v.length ()) {}
> >> +
> >> + template
> >> + array_slice (const vec *v)
> >> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length () : 0)
> >> {}
> >> +
> >> + template
> >> + array_slice (vec *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 *heh = cfun->local_decls;
> array_slice 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::array_slice(vec va_gc>&)?
>6693 | array_slice 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 array_slice::array_slice(const vec&) [with
> T = tree_node*]?
>2264 | array_slice (const vec )
> | ^~~
> /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 arr_slice (*heh);
> |^
>
> [... I trimmed notes about all other candidates...]
>
> Or did you mean something else?
Hmm, so what if you change
template
array_slice (const vec )
: m_base (v.address ()), m_size (v.length ()) {}
to
template
array_slice (const vec )
: m_base (v.address ()), m_size (v.length ()) {}
instead? Thus allow any allocation / placement template arg?
Richard.
> Thanks,
>
> Martin
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
Hi,
On Fri, Aug 26 2022, Richard Biener wrote:
>> Am 26.08.2022 um 18:39 schrieb Martin Jambor :
>>
>> 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 still leads to array_slice,
>> 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
>>
>>* 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 )
>> : m_base (v.address ()), m_size (v.length ()) {}
>>
>> + template
>> + array_slice (vec )
>> +: m_base (v.address ()), m_size (v.length ()) {}
>> +
>> + template
>> + array_slice (const vec *v)
>> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length () : 0) {}
>> +
>> + template
>> + array_slice (vec *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 *heh = cfun->local_decls;
array_slice 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::array_slice(vec&)’
6693 | array_slice 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 array_slice::array_slice(const vec&) [with T = tree_node*]’
2264 | array_slice (const vec )
| ^~~
/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 arr_slice (*heh);
|^
[... I trimmed notes about all other candidates...]
Or did you mean something else?
Thanks,
Martin
Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
> Am 26.08.2022 um 18:39 schrieb Martin Jambor :
>
> 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 still leads to array_slice,
> 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
>
>* 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 )
> : m_base (v.address ()), m_size (v.length ()) {}
>
> + template
> + array_slice (vec )
> +: m_base (v.address ()), m_size (v.length ()) {}
> +
> + template
> + array_slice (const vec *v)
> +: m_base (v ? v->address () : nullptr), m_size (v ? v->length () : 0) {}
> +
> + template
> + array_slice (vec *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?
> iterator begin () { return m_base; }
> iterator end () { return m_base + m_size; }
>
> --
> 2.37.2
>
[PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors
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 still leads to array_slice,
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
* 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 )
: m_base (v.address ()), m_size (v.length ()) {}
+ template
+ array_slice (vec )
+: m_base (v.address ()), m_size (v.length ()) {}
+
+ template
+ array_slice (const vec *v)
+: m_base (v ? v->address () : nullptr), m_size (v ? v->length () : 0) {}
+
+ template
+ array_slice (vec *v)
+: m_base (v ? v->address () : nullptr), m_size (v ? v->length () : 0) {}
+
iterator begin () { return m_base; }
iterator end () { return m_base + m_size; }
--
2.37.2
