Re: [PATCH] Making oprofile multilib friendly

[Maynard J. <may...@us...>]

William Cohen wrote:
> On 06/08/2010 10:10 AM, Maynard Johnson wrote:
>> William Cohen wrote:
> 
>>> The attached patch makes the stl.pat the same across the various architectures. This makes /usr/share/oprofile/stl.pat comply with File Hierarchy Standard (FHS) mentioned in:
>>>
>>> http://fedoraproject.org/wiki/Packaging:Guidelines#Filesystem_Layout
>>>
>>> The attached patch eliminates the need to generate stl.pat from stl.pat.in. Another patch could be made if 
>>
>> Will, I'm not sure what you mean here.  The patch changes stl.pat.in and we still do end up with a generated stl.pat.
> 
> stl.pat.in and stl.pat are identical now. There is no substitution being done to create stl.pat. To make the changes clear I just changed stl.pat.in. Another patch could be written to move stl.pat.in to stl.pat and simplify the configuration.
> 
>>
>>> desired to move stl.pat.in to stl.pat. However, this patch done this way to make it clearer what changes were to stl.pat. Could some one verify that this works on ppc/ppc64 with the following steps:
>>>
>>> ./autogen.sh
>>> ./configure
>>> make
>>> cd libregex
>>> make check
>>
>> I built oprofile + stl patch, both 32-bit and 64-bit on a POWER box and the libregex make check passed in both cases.
>>
> 
> Thanks for testing it out on ppc and ppc64.
> 
>>>
>>> Changelog entry
>>>
>>> 2010-06-06  William Cohen <wc...@re...>
>>>
>>>         * libregex/stl.pat.in: Avoid machine specific configuration.
>>>
>>> Signed-off-by: William Cohen <wc...@re...>
>>>
>>>
>>> -Will
>>>
>>
>> I have difficulty applying these patches that are create with 'cvs diff'.  How do you apply them?
> 
> You should be able to apply the patch by cd into the oprofile directory then doing:
> 
> patch -p0 < /tmp/oprofile-stl2.patch 
> 
>>> @@ -109,11 +111,12 @@
>>>  # form for 2.95/3.2
>>>  # "\<(multi)?map<${typename}, ${typename}, less<\2>>" = "\1map<\2, \8>"
>>>
>>> -"\<bitset<\(@SIZE_T_TYPE@\)(${integer})>" = "bitset<\1>"
>>> +#"\<bitset<\(${size_t_type}\)(${integer})>" = "bitset<\1>"
>>
>> This appears to be a commented-out line that could probably be removed, right?
>>
>> -Maynard
> 
> Yes, that line can be elimianted. I should have reviewed the patch more carefully before sending it. 
> 
> Attached is a patch with the line eliminated.

Will,
I haven't been able to come up with C++ testcases using bitset and stream_iterator where the signatures (as reported via opreport) match the patterns that are changed in your patch.  I wasn't able to see any differences in the "-D smart" opreport output with the various gyrations I tried today.  It seems that many of these patterns are very gcc version-specific, and I wasn't able to find a machine that had anything older than gcc 3.3.3.

I played around a bit with the libregx 'make check'.  I took an stl.pat that was generated from a 32-bit build and stuck it in libregx that was built as 64-bit.  The 'make check' failed, which would be expected.  So, it's a good sign that your patched version of stl.pat works in both 32-bit and 64-bit builds.

I think Philippe Elie was the person most knowledgeable about this stuff, and he's been pretty much out of the picture of late.  AFICT, all of this stl.pat stuff and smart demangling is intended simply to make the output of opreport less cluttered and easier to read.  Unfortunately, I think it's gotten a bit musty and hasn't been well maintained with the latest versions of gcc.

With that said, I think your patch is OK -- so go ahead and commit it.

-Maynard
> 
> -Will

=============================================================================
> Index: libregex/stl.pat.in
> ===================================================================
> RCS file: /cvsroot/oprofile/oprofile/libregex/stl.pat.in,v
> retrieving revision 1.8
> diff -u -r1.8 stl.pat.in
> --- libregex/stl.pat.in	2 Nov 2003 20:06:08 -0000	1.8
> +++ libregex/stl.pat.in	8 Jun 2010 14:30:33 -0000
> @@ -28,6 +28,8 @@
>  # pointer is ugly but we can't add any grouping to not overrun 9 max group
>  # in left pattern rules side..
>  $typename = "(${typename}[ ]*\**|unsigned short[ ]**\**|unsigned int[ ]*\**|unsigned long[ ]*\**|unsigned char[ ]*\**|signed char[ ]*\**|long long[ ]*\**|unsigned long long[ ]*\**|long double[ ]*\**)"
> +$ptrdiff_t_type = "(int|long)"
> +
> 
>  # FIXME: really discussable but simplify output and the next pattern.
>  "\<std::" = ""
> @@ -44,7 +46,7 @@
>  "\<(multi)?map<${typename}, ${typename}, less<\2>>" = "\1map<\2, \8>"
> 
>  "\<bitset<(${integer}), unsigned long>" = "bitset<\1>"
> -"\<([io]stream_iterator)<char, @PTRDIFF_T_TYPE@>" = "\1<char>"
> +"\<([io]stream_iterator)<char, ${ptrdiff_t_type}>" = "\1<char>"
> 
>  # common to all supported gcc version.
>  "\<deque<${typename}, allocator<\1>, 0>" = "deque<\1>"
> @@ -109,11 +111,11 @@
>  # form for 2.95/3.2
>  # "\<(multi)?map<${typename}, ${typename}, less<\2>>" = "\1map<\2, \8>"
> 
> -"\<bitset<\(@SIZE_T_TYPE@\)(${integer})>" = "bitset<\1>"
> +"\<bitset<\(unsigned( long)?\)(${integer})>" = "bitset<\2>"
> 
>  # iterator
> -"\<iterator<(input|output|forward|bidirectional|random)_iterator_tag, ${typename}, (@PTRDIFF_T_TYPE@), \8\*, \8&>" = "iterator<\1_iterator_tag, \2>"
> -"\<([io]stream_iterator)<${typename}, char, char_traits<char>, @PTRDIFF_T_TYPE@>" = "\1<\2>"
> +"\<iterator<(input|output|forward|bidirectional|random)_iterator_tag, ${typename}, (${ptrdiff_t_type}), \8\*, \8&>" = "iterator<\1_iterator_tag, \2>"
> +"\<([io]stream_iterator)<${typename}, char, char_traits<char>, ${ptrdiff_t_type}>" = "\1<\2>"
> 
>  # __gnu_cxx::__normal_iterator are used in two context: basic_string<> and
>  # vector<T> we decay them to string::iterator, vector<T>::iterator