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.

>     ~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.

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)

Reply via email to