Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Paolo Carlini]

Hi,

today I'm seeing an ABI check failure in mainline, x86_64-linux, 11
incompatible symbols: my preliminary analysis shows that the problem is
the recurring one, simple, but annoying: some small functions are not
inlined anymore, thus inadvertently exported (with an old version
number, because of a loose pattern in the linker script). For example:

2
_ZNKSt8ios_base6getlocEv
std::ios_base::getloc() const
version status: incompatible
GLIBCXX_3.4
type: function
status: added

Are there very recent inlining changes?

Thanks,
Paolo.
 

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Richard Guenther]

On Wed, Oct 7, 2009 at 12:24 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> today I'm seeing an ABI check failure in mainline, x86_64-linux, 11
> incompatible symbols: my preliminary analysis shows that the problem is
> the recurring one, simple, but annoying: some small functions are not
> inlined anymore, thus inadvertently exported (with an old version
> number, because of a loose pattern in the linker script). For example:
>
> 2
> _ZNKSt8ios_base6getlocEv
> std::ios_base::getloc() const
> version status: incompatible
> GLIBCXX_3.4
> type: function
> status: added
>
> Are there very recent inlining changes?

Yes.

Richard.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Paolo Carlini]

Richard Guenther wrote:
> Yes.
>   
Thanks ;) I'm going to tighten the patterns asap, cannot hurt. For the
record, on testresults there are no big traces of this issue, this is
puzzling, no idea why some people do not reproduce my problems...

Paolo.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Jan Hubicka]

> > 2
> > _ZNKSt8ios_base6getlocEv
> > std::ios_base::getloc() const
> > version status: incompatible
> > GLIBCXX_3.4
> > type: function
> > status: added
> >
> > Are there very recent inlining changes?
> 
> Yes.

This might be patch I commited today morning.  It would help if you
could just send me testcase showing what function is not getting inlined
and I will investigate.
It changes behaviour WRT COMDAT in some cases, so it might be effect of
this.

Honza
> 
> Richard.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Paolo Carlini]

Hi Honza,
>>> 2
>>> _ZNKSt8ios_base6getlocEv
>>> std::ios_base::getloc() const
>>> version status: incompatible
>>> GLIBCXX_3.4
>>> type: function
>>> status: added
>>>
>>> Are there very recent inlining changes?
>>>       
>> Yes.
>>     
> This might be patch I commited today morning.  It would help if you
> could just send me testcase showing what function is not getting inlined
> and I will investigate.
>   
Thanks. Indeed, I was thinking that even if this kind of problem shows a
weakness in the way we are matching patterns in the linker script, isn't
so *obvious* that quite a few functions are not inlined anymore...

Anyway, as regards *which* specific functions are not inlined, I would
say all the functions which break the ABI test as newly exported symbols
should be checked, like the above, 'std::ios_base::getloc() const'. I'm
attaching below a complete list, from my libstdc++.log, but I would
guess should be easy to reproduce with a GCC tree post your last changes...

Thanks again,
Paolo.

//////////////////

11 incompatible symbols
0
_ZN9__gnu_cxx8__detail9__find_ifIPSt4pairIPNS_16bitmap_allocatorIwE12_Alloc_blockES6_ENS0_12_Functor_RefINS0_12_Ffit_finderIS6_EEEEEET_SD_SD_T0_
std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*
__gnu_cxx::__detail::__find_if<std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*,
__gnu_cxx::__detail::_Functor_Ref<__gnu_cxx::__detail::_Ffit_finder<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>
> >(std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*,
std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*,
__gnu_cxx::__detail::_Functor_Ref<__gnu_cxx::__detail::_Ffit_finder<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>
>)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


1
_ZNSbIwSt11char_traitsIwESaIwEE6assignIPKwEERS2_T_S7_
std::basic_string<wchar_t, std::char_traits<wchar_t>,
std::allocator<wchar_t> >& std::basic_string<wchar_t,
std::char_traits<wchar_t>, std::allocator<wchar_t> >::assign<wchar_t
const*>(wchar_t const*, wchar_t const*)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


2
_ZNKSt8ios_base6getlocEv
std::ios_base::getloc() const
version status: incompatible
GLIBCXX_3.4
type: function
status: added


3
_ZNSs6assignIPKcEERSsT_S3_
std::string& std::string::assign<char const*>(char const*, char const*)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


4
_ZNSs6appendIPKcEERSsT_S3_
std::string& std::string::append<char const*>(char const*, char const*)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


5
_ZNSs6insertIPKcEEvN9__gnu_cxx17__normal_iteratorIPcSsEET_S6_
void std::string::insert<char
const*>(__gnu_cxx::__normal_iterator<char*, std::string>, char const*,
char const*)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


6
_ZNSbIwSt11char_traitsIwESaIwEE6appendIPKwEERS2_T_S7_
std::basic_string<wchar_t, std::char_traits<wchar_t>,
std::allocator<wchar_t> >& std::basic_string<wchar_t,
std::char_traits<wchar_t>, std::allocator<wchar_t> >::append<wchar_t
const*>(wchar_t const*, wchar_t const*)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


7
_ZN9__gnu_cxx8__detail9__find_ifIPSt4pairIPNS_16bitmap_allocatorIcE12_Alloc_blockES6_ENS0_12_Functor_RefINS0_12_Ffit_finderIS6_EEEEEET_SD_SD_T0_
std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*
__gnu_cxx::__detail::__find_if<std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*,
__gnu_cxx::__detail::_Functor_Ref<__gnu_cxx::__detail::_Ffit_finder<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>
> >(std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*,
std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*,
__gnu_cxx::__detail::_Functor_Ref<__gnu_cxx::__detail::_Ffit_finder<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>
>)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


8
_ZN9__gnu_cxx8__detail9__find_ifIPSt4pairIPNS_16bitmap_allocatorIwE12_Alloc_blockES6_ENS0_18_Inclusive_betweenIS6_EEEET_SB_SB_T0_
std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*
__gnu_cxx::__detail::__find_if<std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*,
__gnu_cxx::__detail::_Inclusive_between<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>
>(std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*,
std::pair<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>*,
__gnu_cxx::__detail::_Inclusive_between<__gnu_cxx::bitmap_allocator<wchar_t>::_Alloc_block*>)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


9
_ZN9__gnu_cxx8__detail9__find_ifIPSt4pairIPNS_16bitmap_allocatorIcE12_Alloc_blockES6_ENS0_18_Inclusive_betweenIS6_EEEET_SB_SB_T0_
std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*
__gnu_cxx::__detail::__find_if<std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*,
__gnu_cxx::__detail::_Inclusive_between<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>
>(std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*,
std::pair<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*,
__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>*,
__gnu_cxx::__detail::_Inclusive_between<__gnu_cxx::bitmap_allocator<char>::_Alloc_block*>)
version status: incompatible
GLIBCXX_3.4
type: function
status: added


10
_ZNSbIwSt11char_traitsIwESaIwEE6insertIPKwEEvN9__gnu_cxx17__normal_iteratorIPwS2_EET_SA_
void std::basic_string<wchar_t, std::char_traits<wchar_t>,
std::allocator<wchar_t> >::insert<wchar_t
const*>(__gnu_cxx::__normal_iterator<wchar_t*,
std::basic_string<wchar_t, std::char_traits<wchar_t>,
std::allocator<wchar_t> > >, wchar_t const*, wchar_t const*)
version status: incompatible
GLIBCXX_3.4
type: function
status: added

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Jan Hubicka]

> Anyway, as regards *which* specific functions are not inlined, I would
> say all the functions which break the ABI test as newly exported symbols
> should be checked, like the above, 'std::ios_base::getloc() const'. I'm
> attaching below a complete list, from my libstdc++.log, but I would
> guess should be easy to reproduce with a GCC tree post your last changes...

Hi,
from quick glance at the symbols it seems that the functions are not
that small they would be considered code size win after inlining, so it
might be that inliner just no longer inline them into cold sections
callers.

There is one change concerning COMDAT I made in my patch.  Inliner when
doing size estimates knows that inlining all calls might make out of
line function unnecesary.  This logic was working for COMDAT functions
too (they do disappear from current unit) while after the patch COMDAT
functions are assumed to stay in the unit size after inlining.  I did
this because this logic has an effect that all COMDAT functions that are
called just once in current compilation unit will get inlined (since
producing inline copy while removing offline copy always reduce size)
regardless their size that might lead to uncontrolled program size
growth.

I will see if it reproduces on my setup and if it disappears after this
logic is reverted to original behaviour.
Lets also see how C++ benchmarks will change in behaviour today.

Honza

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Richard Guenther]

On Wed, Oct 7, 2009 at 2:33 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Anyway, as regards *which* specific functions are not inlined, I would
>> say all the functions which break the ABI test as newly exported symbols
>> should be checked, like the above, 'std::ios_base::getloc() const'. I'm
>> attaching below a complete list, from my libstdc++.log, but I would
>> guess should be easy to reproduce with a GCC tree post your last changes...
>
> Hi,
> from quick glance at the symbols it seems that the functions are not
> that small they would be considered code size win after inlining, so it
> might be that inliner just no longer inline them into cold sections
> callers.
>
> There is one change concerning COMDAT I made in my patch.  Inliner when
> doing size estimates knows that inlining all calls might make out of
> line function unnecesary.  This logic was working for COMDAT functions
> too (they do disappear from current unit) while after the patch COMDAT
> functions are assumed to stay in the unit size after inlining.  I did
> this because this logic has an effect that all COMDAT functions that are
> called just once in current compilation unit will get inlined (since
> producing inline copy while removing offline copy always reduce size)
> regardless their size that might lead to uncontrolled program size
> growth.
>
> I will see if it reproduces on my setup and if it disappears after this
> logic is reverted to original behaviour.
> Lets also see how C++ benchmarks will change in behaviour today.

Btw, that new comdat behavior is very well reasonable.  In
whole-program mode it should be the old one though.

Richard.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Jan Hubicka]

> Btw, that new comdat behavior is very well reasonable.  In
> whole-program mode it should be the old one though.

It is another effect of the patch that in whole-program we bring all
comdat functions static except for those having address taken (so the
address must be same from all modules)
I was doing this on pretty-ipa for a while and it helps quite noticeably
for C++.

Honza
> 
> Richard.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Paolo Carlini]

Richard, Jan,

I'm confused. Consider this symbol:

0000000000000000 W
_ZN9__gnu_cxx8__detail9__find_ifIPSt4pairIPNS_16bitmap_allocatorIcE12_Alloc_blockES6_ENS0_12_Functor_RefINS0_12_Ffit_finderIS6_EEEEEET_SD_SD_T0_
version status: incompatible
GLIBCXX_3.4
type: function
status: added

I went multiple times through the linker script and didn't find any
pattern matching it! Can you? What can be possibly going on?

Paolo.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Jan Hubicka]

> Richard, Jan,
> 
> I'm confused. Consider this symbol:
> 
> 0000000000000000 W
> _ZN9__gnu_cxx8__detail9__find_ifIPSt4pairIPNS_16bitmap_allocatorIcE12_Alloc_blockES6_ENS0_12_Functor_RefINS0_12_Ffit_finderIS6_EEEEEET_SD_SD_T0_
> version status: incompatible
> GLIBCXX_3.4
> type: function
> status: added
> 
> I went multiple times through the linker script and didn't find any
> pattern matching it! Can you? What can be possibly going on?

There was bug causing some of abstract (unspecialized) methods to be
mistakely output.  I fixed it this morning, perhaps this is occurence of
this problem?

Honza
> 
> Paolo.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Paolo Carlini]

Hi Honza,
> There was bug causing some of abstract (unspecialized) methods to be
> mistakely output.  I fixed it this morning, perhaps this is occurence of
> this problem?
>   
Thanks for the hint, but I don't think it's that. The regression tester
results are just out for Revision: 152556 and the abi fail is there :(

Really, I have no idea what the heck is going on with those 4 symbols
(probably the other are simpler), I cannot find where the first linker
script part, for baseline (GLIBCXX_3.4), lets them through... This issue
is making me crazy, it even persists if I change that free function to
be a member of bitmap_allocator...

Paolo.

Re: Any very recent inlining changes (libstdc++' ABI check fails in mainline) ?

[Paolo Carlini]

Paolo Carlini wrote:
> Really, I have no idea what the heck is going on with those 4 symbols
> (probably the other are simpler), I cannot find where the first linker
> script part, for baseline (GLIBCXX_3.4), lets them through... This issue
> is making me crazy, it even persists if I change that free function to
> be a member of bitmap_allocator...
>   
Ok, I got it, it's this line:

      std::[p-q]*;

it's because the function *returns* a std::pair... grrrr

Paolo.