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