RFR(XL): 8181449: Fix debug.hpp / globalDefinitions.hpp dependency inversion

[coleen.phillimore at oracle.com]

On 6/16/17 8:33 PM, Kim Barrett wrote:
> Please review this refactoring of debug.hpp and globalDefinitions.hpp
> so that debug.hpp no longer includes globalDefinitions.hpp.  Instead,
> the include dependency is now in the other direction.  Among other
> things, this permits the use of the assert macros by inline functions
> defined in globalDefinitions.hpp.
>
> There are a few functions declared toward the end of debug.hpp that
> now seem somewhat misplaced there.  I'm leaving them there for now,
> but will file a new CR to figure out a better place for them, possibly
> in vmError.  There are a number of additional cleanups for dead code
> and the like that I'll be filing as followups; this change is already
> rather large and I didn't want to add more stuff to it.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8181449
>
> Testing:
> jprt
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8181449/hotspot.00/
>
> The full webrev is somewhat large. However, much of the bulk involves
> either adding #includes to files or moving code from one place to
> another without changing it.  To simplify reviewing the changes, I've
> broken it down into a sequence of patches, each associated with a
> particular bit of the refactoring.  The full change is equivalent to
> applying these patches in the given order.  (Note: I don't know if
> applying a subset gives a working repository.)
>
> (1) http://cr.openjdk.java.net/~kbarrett/8181449/jvm_h/
> a. In preparation for removing the #include of jvm.h from debug.hpp
> (see move_format_buffer webrev), ensured all files that contain
> references to jio_printf variants include jvm.h.  This mostly involved
> adding a #include to lots of files.
>
> b. For a few header files that referenced jio_printf variants, moved
> the function definition from the .hpp to the corresponding .cpp, and
> added #include of jvm.h to the .cpp.
> - macroAssembler_sparc.[ch]pp
> - macroAssembler_x86.[ch]pp
> - macroAssembler_aarch64.[ch]pp

Well that was boring.   I assume that these files adding #include jvm.h 
actually got an error without the include.
>
> (2) http://cr.openjdk.java.net/~kbarrett/8181449/move_format_buffer/
> a. Moved FormatBuffer and related stuff from debug.[ch]pp to new
> formatBuffer.[ch]pp, and updated users to #include the new files.
> This includes moving the #include of jvm.h, which is no longer needed
> by debug.hpp.
>
> b. Made the #include of debug.hpp explicit when already otherwise
> modifying a file that uses assert-like macros, rather than depending
> on indirect inclusion of that file.

Good.
>
> (3) http://cr.openjdk.java.net/~kbarrett/8181449/move_pns/
> a. Moved print_native_stack to VMError class.
> b. Removed unused and undefined pd_obfuscate_location.
> c. Simplified #ifdef PRODUCT guard in ps().

Why can't print_native_stack be in debug.cpp?  I see.  Actually it might 
be better in os.cpp than vmError.cpp.

>
> (4) http://cr.openjdk.java.net/~kbarrett/8181449/target_macros/
>
> a. Moved / combined definitions of BREAKPOINT macro from
> globalDefinitions_*.hpp to new breakpoint.hpp.
>
> b. Deleted all definitions of unused DEBUG_EXCEPTION macro.
>
> c. Moved gcc-specific ATTRIBUTE_PRINTF, pragma macros, and friends
> from globalDefinitions_gcc.hpp to new compilerWarnings.hpp.  Also
> moved the default definitions for those macros from
> globalDefinitions.hpp to compilerWarnings.hpp.
>
> d. Added TARGET_COMPILER_HEADER[_INLINE] macros, similar to the
> CPU/OS/OS_CPU/_HEADER[_INLINE] macros, for including files based on
> new INCLUDE_SUFFIX_TARGET_COMPILER macro provided by the build system.

Ok.

>
> (5) http://cr.openjdk.java.net/~kbarrett/8181449/flip_depend/
>
> a. Changed globalDefinitions.hpp to #include debug.hpp, rather than
> the other way around.
>
> b. Changed globals.hpp to #include globalDefinitions.hpp rather than
> debug.hpp, since it actually needs to former and not the latter.
>
> c. Changed jvmci_globals.cpp to #include jvm.h, since it's no longer
> being indirectly included via an indirect include of debug.hpp that
> was including globalDefinitions.hpp.
>
> d. Moved printf-style formatters earlier in globalDefinitions.hpp, so
> they can be used in assert messages in this file.
>
> e. In globalDefinitions.hpp, changed some #ifdef ASSERT blocks of
> conditional calls to basic_fatal to instead use assert.  While doing
> so, made the error messages more informative.

Also looks good.

>
> In addition to globals.hpp, there are about 90 files that #include
> debug.hpp but not globalDefinitions.hpp.  The few changes mentioned
> were sufficient to fix missing indirect includes resulting from
> debug.hpp no longer including globalDefinitions.hpp.  There could be
> additional problems with platforms not supported by Oracle though.
>
> There are also about 40 files which directly include debug.hpp but
> don't appear to use any of the assertion macros.
>
Hm, might be worth another round to clean these up too.

These changes look good though.  Thank you for the cleanup!!  It wasn't 
as large as I expected (at least this round).

Coleen