On Wed, Feb 12, 2014 at 4:21 PM, Trevor Saunders <tsaund...@mozilla.com> wrote:
> On Wed, Feb 12, 2014 at 03:09:17PM +0100, Richard Biener wrote:
>>
>> The following aims to improve code generation for loops like
>> those in tree-ssa-alias.c:
>>
>> auto_vec<tree, 16> component_refs1;
>> auto_vec<tree, 16> component_refs2;
>>
>> /* Create the stack of handled components for REF1. */
>> while (handled_component_p (ref1))
>> {
>> component_refs1.safe_push (ref1);
>> ref1 = TREE_OPERAND (ref1, 0);
>> }
>>
>> It does so by making sure the vector itself doesn't necessarily
>> escape (to vec.c:calculate_allocation) and by simplifying the
>> way we identify a vector using auto storage. In the end this
>> results in us doing better CSE. But not SRA unfortunately,
>> as we have
>>
>> component_refs1.m_header.m_alloc = 16;
>> component_refs1.m_header.m_using_auto_storage = 1;
>> component_refs1.m_header.m_num = 0;
>> component_refs1.D.56915.m_vec = &component_refs1.m_header;
>>
>> which is what keeps component_refs1 address-taken (no easy
>> way out of that I fear).
>>
>> It also gets rid of the aliasing-suspicious
>>
>> this->m_vec = reinterpret_cast<vec<T, va_heap, vl_embed> *>
>> (&m_header);
>>
>> and the friend declaration in class auto_vec.
>
> over all I like this, only two small comments
>
>> (vec_prefix::m_has_auto_buf): Rename to ...
>> (vec_prefix::m_using_auto_storage): ... this.
>
> One optimization that might be worth doing is if a vector is cleared
> reseting m_vec to point at the internal storage in which case the old
> name would be more accurate, on the other hand its probably rare that's
> useful, and doing it this way is simpler.
Yeah, I thought about this but disregarded it as not-the-common case.
>> ~auto_vec ()
>> --- 1250,1257 ----
>> public:
>> auto_vec ()
>> {
>> ! m_auto.embedded_init (MAX (N, 2), 0, 1);
>> ! this->m_vec = &m_auto;
>> }
>>
>> ~auto_vec ()
>> *************** public:
>> *** 1246,1255 ****
>> }
>>
>> private:
>> ! friend class vec<T, va_heap, vl_ptr>;
>> !
>> ! vec_prefix m_header;
>> ! T m_data[N];
>> };
>>
>> /* auto_vec is a sub class of vec whose storage is released when it is
>> --- 1260,1267 ----
>> }
>>
>> private:
>> ! vec<T, va_heap, vl_embed> m_auto;
>> ! T m_data[MAX (N - 1, 1)];
>
> why is the MAX() needed? auto_vec<T, 0> is specialized below so someone
> would need to write auto_vec<T, -x> for N - 1 to be less than 1 which is
> crazy, and seems like shouldn't be allowed to compile.
It's for the quite common auto_vec<T, 1> case where T m_data[N-1]
doesn't work (zero-sized array is a GNU extension). I've had the
variant of simply allocating N + 1 elements (and thus using
m_data[N]) but then the above is closer to what people expect (but
sets the minimum auto size to two elements).
Alternatively one could have templated vec<T, va_heap, vl_embed>
itself with a size for its m_data member. Or specialize auto_vec
for N == 1 and leave out the m_data member for that.
I considered both not worth the trouble ;)
If you prefer using m_data[N] and thus allocating one more element
than requested in auto storage I can do that as well.
Thanks,
Richard.
> thanks!
>
> Trev
>
>> };
>>
>> /* auto_vec is a sub class of vec whose storage is released when it is
>> *************** template<typename T>
>> *** 1396,1402 ****
>> inline bool
>> vec<T, va_heap, vl_ptr>::reserve (unsigned nelems, bool exact
>> MEM_STAT_DECL)
>> {
>> ! if (!nelems || space (nelems))
>> return false;
>>
>> /* For now play a game with va_heap::reserve to hide our auto storage if
>> any,
>> --- 1408,1414 ----
>> inline bool
>> vec<T, va_heap, vl_ptr>::reserve (unsigned nelems, bool exact
>> MEM_STAT_DECL)
>> {
>> ! if (space (nelems))
>> return false;
>>
>> /* For now play a game with va_heap::reserve to hide our auto storage if
>> any,
>> *************** vec<T, va_heap, vl_ptr>::release (void)
>> *** 1462,1468 ****
>>
>> if (using_auto_storage ())
>> {
>> ! static_cast<auto_vec<T, 1> *> (this)->m_header.m_num = 0;
>> return;
>> }
>>
>> --- 1474,1480 ----
>>
>> if (using_auto_storage ())
>> {
>> ! m_vec->m_vecpfx.m_num = 0;
>> return;
>> }
>>
>> *************** template<typename T>
>> *** 1705,1716 ****
>> inline bool
>> vec<T, va_heap, vl_ptr>::using_auto_storage () const
>> {
>> ! if (!m_vec->m_vecpfx.m_has_auto_buf)
>> ! return false;
>> !
>> ! const vec_prefix *auto_header
>> ! = &static_cast<const auto_vec<T, 1> *> (this)->m_header;
>> ! return reinterpret_cast<vec_prefix *> (m_vec) == auto_header;
>> }
>>
>> #if (GCC_VERSION >= 3000)
>> --- 1717,1723 ----
>> inline bool
>> vec<T, va_heap, vl_ptr>::using_auto_storage () const
>> {
>> ! return m_vec->m_vecpfx.m_using_auto_storage;
>> }
>>
>> #if (GCC_VERSION >= 3000)
