RFR (S) 8182848: Some functions misplaced in debug.hpp

[coleen.phillimore at oracle.com]

On 6/28/17 12:45 PM, Stefan Karlsson wrote:
> Hi Coleen,
>
> http://cr.openjdk.java.net/~coleenp/8182848.01/webrev/src/share/vm/runtime/frame.hpp.udiff.html
> + DEBUG_ONLY(void pd_ps();) // platform dependent frame printing
>
> Shouldn't this be guarded by NOT_PRODUCT when all implementations are 
> guarded by #ifndef PRODUCT ?

Yes it should be.  I should build optimized (if it still builds).
>
> Time again to consider removing the optimized target ;)
>
> http://cr.openjdk.java.net/~coleenp/8182848.01/webrev/src/cpu/sparc/vm/frame_sparc.cpp.frames.html
>
> In this file you didn't add frame::pd_ps() after one of the frame 
> constructors. Maybe move it to line 407 to be consistent with the 
> other platform files?

ok, done.

>
> I know you didn't change this, but it's weird that findpc is declared as:
> 790 extern "C" void findpc(int x);
> when all other platforms use intptr_t x.

Yes, good spot.  Fixed declaration and use without cast below.
>
> and the code below casts to intptr_t:
> 803 findpc((intptr_t)pc);
> http://cr.openjdk.java.net/~coleenp/8182848.01/webrev/src/cpu/x86/vm/frame_x86.cpp.frames.html
>
> 684 void frame::pd_ps() {}
> 685
>   686 #endif
>
> You inserted a stray blank line at 685, that is not present in the 
> other platform files.

gone! fixed.
>
> Other than that, this looks good.

Thank you for the code review!
Coleen

>
> Thanks,
> StefanK
> On 2017-06-28 17:38, coleen.phillimore at oracle.com wrote:
>> Summary: moved to vmError.hpp,cpp where they seemed more appropriate
>>
>> Moved the function pd_ps() into frame_sparc.cpp eliminating 
>> debug_cpu.cpp files.   You can pick a better name for this if you want.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8182848.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8182848
>>
>> Tested with JPRT.
>>
>> Thanks,
>> Coleen
>>
>