[16] RFR(S): 8251093: Improve C1 register allocator logging and debugging support

[Christian Hagedorn]

Hi Vladimir

Yes, you're right, these should be changed into ASSERT and DEBUG().

I'm wondering though if these ifdefs are even required for if-blocks 
inside methods?

Isn't, for example, this if-block:

#ifndef PRODUCT
         if (TraceLinearScanLevel >= 2) {
           tty->print_cr("killing XMMs for trig");
         }
#endif

removed anyways when the flag is set to < 2 (which is statically known 
and thus would allow this entire block to be removed)? Or does it make a 
difference by explicitly guarding it with an ifdef?

Best regards,
Christian

On 14.08.20 20:09, Vladimir Kozlov wrote:
> One note. Most of the code is guarded by #ifndef PRODUCT.
> 
> But the flag is available only in DEBUG build:
>    develop(intx, TraceLinearScanLevel, 0,
> 
> Should we use #ifdef ASSERT and DEBUG() instead?
> 
> Thanks,
> Vladimir
> 
> On 8/14/20 5:10 AM, Christian Hagedorn wrote:
>> Hi
>>
>> Please review the following enhancement for C1:
>> https://bugs.openjdk.java.net/browse/JDK-8251093
>> http://cr.openjdk.java.net/~chagedorn/8251093/webrev.00/
>>
>> While I was working on JDK-8249603 [1], I added some additional 
>> debugging and logging code which helped to figure out what was going 
>> on. I think it would be useful to have this code around for the 
>> analysis of future C1 register allocator bugs.
>>
>> This RFE adds (everything non-product code):
>> - find_interval(number): Can be called like that from gdb anywhere to 
>> find an interval with the given number.
>> - Interval::print_children()/print_parent(): Useful when debugging 
>> with gdb to quickly show the split children and parent.
>> - LinearScan::print_reg_num(number): Prints the register or stack 
>> location for this register number. This is useful in some places 
>> (logging with TraceLinearScanLevel set) where it just printed a number 
>> which first had to be manually looked up in other logs.
>>
>> I additionally did some cleanup of the touched code.
>>
>> We could additionally split the TraceLinearScanLevel flag into 
>> separate flags related to the different phases of the register 
>> allocation algorithm. It currently just prints too much details on the 
>> higher levels. You often find yourself being interested in a specific 
>> part of the algorithm and only want to know more details there. To 
>> achieve that you now you have to either handle all the noise or 
>> manually disable/enable other logs. We could file an RFE to clean this 
>> up if it's worth the effort - given that there are not many new issues 
>> filed for C1 register allocation today.
>>
>> Thank you!
>>
>> Best regards,
>> Christian
>>
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8251093
>>