RFR(XS): 8046611: Build errors with gcc on sparc/fastdebug

[David Holmes]

On 19/06/2014 11:21 AM, Christian Thalinger wrote:
> Old, long-time unused debug methods are worthless.  I’d suggested to just remove these methods.

Sometimes when developing code, or debugging it, it is very useful to 
add additional introspection functions to aid with testing and 
debugging. Past developers have left these in place in various bits of 
code throughout the VM. As time progresses people who don't use them see 
them simply as dead code to be removed.

While I can see the utility in such methods in general (not commenting 
on these specific ones) I also see the reality that over time new 
developers don't even realize the code exists and so tend to roll their 
own introspection tools as needed.

So I would not fight hard for this code to stay. But it isn't my code 
and I don't know that it is unused by other developers.

Cheers,
David

> On Jun 17, 2014, at 9:08 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
>
>>
>> I can file an RFE on that if we feel that it's worth following up on.
>>
>> Does anybody have comments on the rest of the new webrev?
>>
>> Cheers,
>> Mikael
>>
>>> On Jun 16, 2014, at 5:48, Marcus Lagergren <marcus.lagergren at oracle.com> wrote:
>>>
>>> Maybe we could introduce an unused define, if there’s no platform independent way of doing it in C.
>>>
>>> GCC has something line  __attribute__ ((deprecated)), and there’s #pragma unused on some platforms…
>>>
>>> /M
>>>
>>>
>>>> On 13 Jun 2014, at 08:08, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
>>>>
>>>>
>>>>> On 2014-06-11 18:36, David Holmes wrote:
>>>>> Hi Mikael,
>>>>>
>>>>>> On 12/06/2014 5:47 AM, Mikael Vidstedt wrote:
>>>>>>
>>>>>> Please review the below small change which fixes a couple of build
>>>>>> errors when building the sparc files with gcc.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8046611
>>>>>> Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8046611/webrev.00/webrev
>>>>>>
>>>>>> Without the fix the following two errors are seen:
>>>>>>
>>>>>> hotspot/src/cpu/sparc/vm/frame_sparc.cpp:442: error: ‘frame
>>>>>> nth_sender(int)’ defined but not used
>>>>>> hotspot/src/share/vm/runtime/safepoint.cpp:771: error: ‘void
>>>>>> print_me(intptr_t*, intptr_t*, bool*)’ defined but not used
>>>>>>
>>>>>> The functions in question are debug functoins declared static. Assuming
>>>>>> we'll want to keep them around the fix is to simply remove the static
>>>>>> keyword from them.
>>>>>
>>>>> This seems so wrong. Changing them to non-static simply stops the compiler from being able to deduce whether they are used or not. But can they actually be used if declared non-static?
>>>>>
>>>>> Can we not tag them some way to tell the compiler to ignore them?
>>>>>
>>>>> Debugging functions can be useful even if rarely used. But no one will know they exist if they aren't in the code so I'm wary of simply deleting everything of that nature.
>>>>
>>>> I don't know of any way to reliably tag functions as used apart from removing the static keyword.
>>>>
>>>> I totally redid the safepoint.cpp debug function(s) part of the change. First of all they no longer need to handle the 32-bit SPARC case, so that part of the code goes away. That also enables removing the print_longs function (it will be the same as print_ptrs), and I did some additional clean up on top of that. Verified that it works by manually enabling the const bool and looked at the output.
>>>>
>>>> I'm not sure what to do about the nth_sender function though, mostly because I don't know how people expect to use it. We could for example move it to utilities/debug.cpp and export it as the other debug function there, which is what I've done in the below webrev just so that we can see what it would look like. Gladly taking feedback on it though, especially from people actually having used the function in recent times. If it hasn't been used recently it really should go away...
>>>>
>>>> Updated webrev:
>>>>
>>>> http://cr.openjdk.java.net/~mikael/webrevs/8046611/webrev.01/webrev/
>>>>
>>>> Cheers,
>>>> Mikael
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Cheers,
>>>>>> Mikael
>>>
>