RFR (S) 8182848: Some functions misplaced in debug.hpp
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Wed Jun 28 18:41:22 UTC 2017
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 ?
>
> Time again to consider removing the optimized target ;)
I don't know how to build the optimized target anymore. yes, we should
remove it!!
Coleen
>
> 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?
>
> 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.
>
> 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.
>
> Other than that, this looks good.
>
> 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
>>
>
More information about the hotspot-dev
mailing list