RFR (S) 8182848: Some functions misplaced in debug.hpp
Stefan Karlsson
stefan.karlsson at oracle.com
Wed Jun 28 16:45:33 UTC 2017
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 ;)
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