| Message ID | 20190114081942.9088-2-andi@firstfloor.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] Lower sampling rate for autofdo bootstrap | expand |
On Mon, Jan 14, 2019 at 9:20 AM Andi Kleen <andi@firstfloor.org> wrote: > > From: Andi Kleen <ak@linux.intel.com> > > autoprofiledbootstrap fails currently with > > In file included from ../../gcc/gcc/hash-table.h:236, > from ../../gcc/gcc/coretypes.h:440, > from ../../gcc/gcc/ipa-devirt.c:110: > In static member function 'static void va_heap::release(vec<T, va_heap, vl_embed>*&) [with T = tree_node*]', > inlined from 'void vec<T>::release() [with T = tree_node*]' at ../../gcc/gcc/vec.h:1679:20, > inlined from 'auto_vec<T, N>::~auto_vec() [with T = tree_node*; long unsigned int N = 8]' at ../../gcc/gcc/vec.h:1436:5, > inlined from 'vec<cgraph_node*> possible_polymorphic_call_targets(tree, long int, ipa_polymorphic_call_context, bool*, void**, bool)' at ../../gcc/gcc/ipa-devirt.c:3099:22: > ../../gcc/gcc/vec.h:311:10: error: attempt to free a non-heap object 'bases_to_consider' [-Werror=free-nonheap-object] > 311 | ::free (v); > | ~~~~~~~^~~ > ../../gcc/gcc/vec.h:311:10: error: attempt to free a non-heap object 'bases_to_consider' [-Werror=free-nonheap-object] > cc1plus: all warnings being treated as errors > > The problem is that auto_vec uses a variable to keep track if the vector > is on the heap or auto. Normally this gets constant resolved, but only > when the right functions are inlined. With autofdo for some reason > the compiler decides to not inline these vec functions, even though > they are marked as "inline" > > Mark them as ALWAYS_INLINE instead. This might fix your case but I think it only papers over the issue. Consider auto_vec<...> vec; not-inlined-foo (vec); where the function can end up re-allocating the vector. I think the more appropriate fix is to add #pragma GCC diagnostic pus/pop and ignored "-Wfree-nonheap-object" around the inline function (and hope for the best that works in the inlined contexts...) Richard. > gcc/: > > 2019-01-14 Andi Kleen <ak@linux.intel.com> > > * vec.h (using_auto_storage, release): Mark as ALWAYS_INLINE. > --- > gcc/vec.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gcc/vec.h b/gcc/vec.h > index 407269c5ad3..1f5b78b1fac 100644 > --- a/gcc/vec.h > +++ b/gcc/vec.h > @@ -1664,7 +1664,7 @@ vec<T, va_heap, vl_ptr>::create (unsigned nelems MEM_STAT_DECL) > /* Free the memory occupied by the embedded vector. */ > > template<typename T> > -inline void > +ALWAYS_INLINE void > vec<T, va_heap, vl_ptr>::release (void) > { > if (!m_vec) > @@ -1940,7 +1940,7 @@ vec<T, va_heap, vl_ptr>::reverse (void) > } > > template<typename T> > -inline bool > +ALWAYS_INLINE bool > vec<T, va_heap, vl_ptr>::using_auto_storage () const > { > return m_vec->m_vecpfx.m_using_auto_storage; > -- > 2.19.1 >
On Mon, Jan 14, 2019 at 4:20 PM Andi Kleen <andi@firstfloor.org> wrote: > > From: Andi Kleen <ak@linux.intel.com> > > autoprofiledbootstrap fails currently with > > In file included from ../../gcc/gcc/hash-table.h:236, > from ../../gcc/gcc/coretypes.h:440, > from ../../gcc/gcc/ipa-devirt.c:110: > In static member function 'static void va_heap::release(vec<T, va_heap, vl_embed>*&) [with T = tree_node*]', > inlined from 'void vec<T>::release() [with T = tree_node*]' at ../../gcc/gcc/vec.h:1679:20, > inlined from 'auto_vec<T, N>::~auto_vec() [with T = tree_node*; long unsigned int N = 8]' at ../../gcc/gcc/vec.h:1436:5, > inlined from 'vec<cgraph_node*> possible_polymorphic_call_targets(tree, long int, ipa_polymorphic_call_context, bool*, void**, bool)' at ../../gcc/gcc/ipa-devirt.c:3099:22: > ../../gcc/gcc/vec.h:311:10: error: attempt to free a non-heap object 'bases_to_consider' [-Werror=free-nonheap-object] > 311 | ::free (v); > | ~~~~~~~^~~ > ../../gcc/gcc/vec.h:311:10: error: attempt to free a non-heap object 'bases_to_consider' [-Werror=free-nonheap-object] > cc1plus: all warnings being treated as errors > > The problem is that auto_vec uses a variable to keep track if the vector > is on the heap or auto. Normally this gets constant resolved, but only > when the right functions are inlined. With autofdo for some reason > the compiler decides to not inline these vec functions, even though > they are marked as "inline" A comment not closely related to this patch. We observed the same inline behavior in which perf data is inadequate, sometime it has non-trivial impact on kernel compilation. We have patch fall back to guessed profile count if the profiled count is of low quality. Will send it out in GCC10. Thanks, bin > > Mark them as ALWAYS_INLINE instead. > > gcc/: > > 2019-01-14 Andi Kleen <ak@linux.intel.com> > > * vec.h (using_auto_storage, release): Mark as ALWAYS_INLINE. > --- > gcc/vec.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gcc/vec.h b/gcc/vec.h > index 407269c5ad3..1f5b78b1fac 100644 > --- a/gcc/vec.h > +++ b/gcc/vec.h > @@ -1664,7 +1664,7 @@ vec<T, va_heap, vl_ptr>::create (unsigned nelems MEM_STAT_DECL) > /* Free the memory occupied by the embedded vector. */ > > template<typename T> > -inline void > +ALWAYS_INLINE void > vec<T, va_heap, vl_ptr>::release (void) > { > if (!m_vec) > @@ -1940,7 +1940,7 @@ vec<T, va_heap, vl_ptr>::reverse (void) > } > > template<typename T> > -inline bool > +ALWAYS_INLINE bool > vec<T, va_heap, vl_ptr>::using_auto_storage () const > { > return m_vec->m_vecpfx.m_using_auto_storage; > -- > 2.19.1 >
diff --git a/gcc/vec.h b/gcc/vec.h index 407269c5ad3..1f5b78b1fac 100644 --- a/gcc/vec.h +++ b/gcc/vec.h @@ -1664,7 +1664,7 @@ vec<T, va_heap, vl_ptr>::create (unsigned nelems MEM_STAT_DECL) /* Free the memory occupied by the embedded vector. */ template<typename T> -inline void +ALWAYS_INLINE void vec<T, va_heap, vl_ptr>::release (void) { if (!m_vec) @@ -1940,7 +1940,7 @@ vec<T, va_heap, vl_ptr>::reverse (void) } template<typename T> -inline bool +ALWAYS_INLINE bool vec<T, va_heap, vl_ptr>::using_auto_storage () const { return m_vec->m_vecpfx.m_using_auto_storage;
From: Andi Kleen <ak@linux.intel.com> autoprofiledbootstrap fails currently with In file included from ../../gcc/gcc/hash-table.h:236, from ../../gcc/gcc/coretypes.h:440, from ../../gcc/gcc/ipa-devirt.c:110: In static member function 'static void va_heap::release(vec<T, va_heap, vl_embed>*&) [with T = tree_node*]', inlined from 'void vec<T>::release() [with T = tree_node*]' at ../../gcc/gcc/vec.h:1679:20, inlined from 'auto_vec<T, N>::~auto_vec() [with T = tree_node*; long unsigned int N = 8]' at ../../gcc/gcc/vec.h:1436:5, inlined from 'vec<cgraph_node*> possible_polymorphic_call_targets(tree, long int, ipa_polymorphic_call_context, bool*, void**, bool)' at ../../gcc/gcc/ipa-devirt.c:3099:22: ../../gcc/gcc/vec.h:311:10: error: attempt to free a non-heap object 'bases_to_consider' [-Werror=free-nonheap-object] 311 | ::free (v); | ~~~~~~~^~~ ../../gcc/gcc/vec.h:311:10: error: attempt to free a non-heap object 'bases_to_consider' [-Werror=free-nonheap-object] cc1plus: all warnings being treated as errors The problem is that auto_vec uses a variable to keep track if the vector is on the heap or auto. Normally this gets constant resolved, but only when the right functions are inlined. With autofdo for some reason the compiler decides to not inline these vec functions, even though they are marked as "inline" Mark them as ALWAYS_INLINE instead. gcc/: 2019-01-14 Andi Kleen <ak@linux.intel.com> * vec.h (using_auto_storage, release): Mark as ALWAYS_INLINE. --- gcc/vec.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)