[16] RFR(S): 8251093: Improve C1 register allocator logging and debugging support
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Aug 18 15:41:31 UTC 2020
c1_Compilation.hpp: looks like both versions of allocator() do the same thing.
I suggest to build with configure --with-debug-level=optimized to check that NOT_PRODUCT can be built with these changes.
Thanks,
Vladimir
On 8/18/20 6:16 AM, Christian Hagedorn wrote:
> Hi Vladimir
>
> On 17.08.20 19:36, Vladimir Kozlov wrote:
>> On 8/17/20 12:44 AM, Christian Hagedorn wrote:
>>> 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?
>>
>> You are right. It could be statically removed. But we keep #ifdef sometimes to indicate that code is executed only in
>> debug build because we don't always remember type of a flag.
>
> I see, that makes sense. I updated my patch and left the ifdefs there but changed them to ASSERT. I also updated other
> ifdefs belonging to TraceLinearScanLevel appropriately.
>
> http://cr.openjdk.java.net/~chagedorn/8251093/webrev.01/
>
> Best regards,
> Christian
>
>>
>> Thanks,
>> Vladimir K
>>
>>>
>>> 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
>>>>>
More information about the hotspot-compiler-dev
mailing list