Re: [PATCH 1/2] vec: Add array_slice constructors from non-const and gc vectors

[Martin Jambor <mjambor@suse.cz>]

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 <mjambor@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  <mjambor@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?

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