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

[Stefan Karlsson]

On 2017-06-22 22:54, Kim Barrett wrote:
>> On Jun 22, 2017, at 2:49 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> On 2017-06-22 02:29, Kim Barrett wrote:
>>>>> (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().
>>>> You removed the call to p->trace_stack(). Was that intentional?
>>>>
>>>>    if (p->has_last_Java_frame()) {
>>>>      // If the last_Java_fp is set we are in C land and
>>>>      // can call the standard stack_trace function.
>>>> -#ifdef PRODUCT
>>>>      p->print_stack();
>>>>    } else {
>>>> +#ifdef PRODUCT
>>>>      tty->print_cr("Cannot find the last Java frame, printing stack disabled.");
>>>> #else // !PRODUCT
>>>> -    p->trace_stack();
>>>> -  } else {
>>> I kept getting confused by this code while trying to figure out what
>>> to do with pd_ps, which is why I changed it.  Seems I was even more
>>> confused than I thought, since I failed to see the difference between
>>> p->print_stack() and p->trace_stack().  Well spotted.  I've reverted
>>> this change.
>> OK. I wonder if this code would have been easier to read with to #ifdef PRODUCT sections.

to -> two :)

> I looked at that before simply reverting, and it was still pretty ugly.  And I’d have
> needed to figure out how to test.  I shouldn’t have touched this code as part of
> this set of changes in the first place; I’m just glad you spotted the mistake.

OK.

>
>>>> http://cr.openjdk.java.net/~kbarrett/8181449/target_macros/src/share/vm/utilities/globalDefinitions_gcc.hpp.udiff.html
>>>>
>>>> Maybe also get rid of the following line:
>>>> //----------------------------------------------------------------------------------------------------
>>>> -// Debugging
>>> Gone now.
>> I wasn't particularly clear on this request, and you cut out the following part of my mail:
>> "or get rid of the following stray new line so that the code in the different globalDefinitions files are more consistent.”
> Yeah, I misunderstood what you were asking for.
>
> What I've ended up doing is deleting all the "//--- ... ---" lines in
> the globalDefinitions_xxx.hpp files.  There weren't very many, all in
> this general vicinity, and just making the one near the nanness comment
> consistent would have left other similarly odd file to file variations.
>
>> http://cr.openjdk.java.net/~kbarrett/8181449/hotspot.01.inc/src/share/vm/utilities/globalDefinitions.hpp.udiff.html
>>
>>   31
>>   32 #include TARGET_COMPILER_HEADER(utilities/globalDefinitions)
>>   33 // Defaults for macros that might be defined per compiler.
>>   34 #ifndef NOINLINE
>>   35 #define NOINLINE
>>   36 #endif
>>
>> Would you mind adding a newline between 32 and 32 before pushing?

32 and 33 ...

> Done.
>
> I also updated the copyrights for globalDefinitions.hpp and
> macros.hpp, which got lost somewhere in the patch splitting.

Thanks,
StefanK

>
>