PR middle-end/51411: handle transaction_safe virtual inlined methods

> Without the patch, it is ok.
> If I remove completely this part:
> if (DECL_DECLARED_INLINE_P (new_decl) && DECL_EXTERNAL (new_decl))
> {
> DECL_EXTERNAL (new_decl) = 0;
> TREE_PUBLIC (new_decl) = 0;
> }
> It ends with *only* one undefined symbol:
> src/UnitsSkins.o: In function
> `_ZGTtStltIcSt11char_traitsIcESaIcEEbRKSbIT_T0_T1_ES8_':
> ./include/c++/4.7.0/bits/basic_string.h:2568: undefined reference to
> `_ZGTtNKSs7compareERKSs'
>
> $ c++filt _ZNKSs7compareERKSs
> std::basic_string<char, std::char_traits<char>, std::allocator<char>
>  >::compare(std::basic_string<char, std::char_traits<char>,
> std::allocator<char> > const&) const
>
> I am not sure the way to give you useful information.

This is actually good, because we were missing a testcase for the 
original patch that zapped DECL_EXTERNAL and TREE_PUBLIC.  Do you mind 
reducing a testcase for me so we don't regress with this patch?  I know 
it'll be a bit challenging...

Perhaps we can try unsetting (hee hee hee) DECL_WEAK as originally 
proposed.  Does this fix your problem?

On 12/14/2011 07:56 AM, Aldy Hernandez wrote:
>
>> Without the patch, it is ok.
>> If I remove completely this part:
>> if (DECL_DECLARED_INLINE_P (new_decl) && DECL_EXTERNAL (new_decl))
>> {
>> DECL_EXTERNAL (new_decl) = 0;
>> TREE_PUBLIC (new_decl) = 0;
>> }
>> It ends with *only* one undefined symbol:
>> src/UnitsSkins.o: In function
>> `_ZGTtStltIcSt11char_traitsIcESaIcEEbRKSbIT_T0_T1_ES8_':
>> ./include/c++/4.7.0/bits/basic_string.h:2568: undefined reference to
>> `_ZGTtNKSs7compareERKSs'
>>
>> $ c++filt _ZNKSs7compareERKSs
>> std::basic_string<char, std::char_traits<char>, std::allocator<char>
>> >::compare(std::basic_string<char, std::char_traits<char>,
>> std::allocator<char> > const&) const
>>
>> I am not sure the way to give you useful information.
>
> This is actually good, because we were missing a testcase for the
> original patch that zapped DECL_EXTERNAL and TREE_PUBLIC. Do you mind
> reducing a testcase for me so we don't regress with this patch? I know
> it'll be a bit challenging...

See attached. Probably not completely reduced...
A call to _ZGTtNKSs7compareERKSs is done but the clone is not created 
(the original function _ZNKSs7compareERKSs is defined in libstdc++). I 
am not sure how to add the rule for the testsuite.

> Perhaps we can try unsetting (hee hee hee) DECL_WEAK as originally
> proposed. Does this fix your problem?

Yep it does! ;)

Patrick.

>
> Index: trans-mem.c
> ===================================================================
> --- trans-mem.c (revision 182290)
> +++ trans-mem.c (working copy)
> @@ -4259,6 +4259,7 @@ ipa_tm_create_version (struct cgraph_nod
> {
> DECL_EXTERNAL (new_decl) = 0;
> TREE_PUBLIC (new_decl) = 0;
> + DECL_WEAK (new_decl) = 0;
> }
>
> tree_function_versioning (old_decl, new_decl, NULL, false, NULL,
namespace std {
template<typename _CharT> struct char_traits;

template<typename _Tp> class allocator {
};

template<typename _Tp> struct less {
    bool operator()(const _Tp& __x, const _Tp& __y) const {
        return __x < __y;
    }
};

template <typename _Key, typename _Compare = std::less<_Key> > class map {
public:
    _Compare _M_key_compare;
    bool find(const _Key& __x) {
        return _M_key_compare(__x, __x);
    }
};

template<typename _CharT, typename _Traits = char_traits<_CharT>, typename _Alloc = allocator<_CharT> > class basic_string {
public:
    bool compare(const basic_string& __str) const {
        return 0;
    }
};

typedef basic_string<char> string;

template<typename _CharT, typename _Traits>
inline bool operator<(const basic_string<_CharT, _Traits>& __lhs, const basic_string<_CharT, _Traits>& __rhs) {
    return __lhs.compare(__rhs);
}

extern template class basic_string<char>;

}

std::map<std::string> units;

__attribute__((transaction_callable))
void get(const std::string &name) {
    units.find(name);
}

Richard, Jason, are you ok with just unsetting DECL_WEAK?

I will come up with a suitable testcase for Patrick's case.

>> Perhaps we can try unsetting (hee hee hee) DECL_WEAK as originally
>> proposed. Does this fix your problem?
>
> Yep it does! ;)
>
> Patrick.
>
>>
>> Index: trans-mem.c
>> ===================================================================
>> --- trans-mem.c (revision 182290)
>> +++ trans-mem.c (working copy)
>> @@ -4259,6 +4259,7 @@ ipa_tm_create_version (struct cgraph_nod
>> {
>> DECL_EXTERNAL (new_decl) = 0;
>> TREE_PUBLIC (new_decl) = 0;
>> + DECL_WEAK (new_decl) = 0;
>> }
>>
>> tree_function_versioning (old_decl, new_decl, NULL, false, NULL,
>

On 12/15/2011 09:22 AM, Aldy Hernandez wrote:
> Richard, Jason, are you ok with just unsetting DECL_WEAK?

For now, yes.

> I will come up with a suitable testcase for Patrick's case.

Once there's a testcase, we can figure out why the other patch didn't work.

Jason