RFR: JDK-8217909: Make unused r12 register (without compressed oops) available to regalloc in C2

[Nils Eliasson]

HI,

The reg_classes that was introduced with PreservedFramePointer 
(*_with_rbp, *_no_rbp) is still there in x86_x64.ad. How about handling 
rbp in the same way as you suggest for r12 (patching the mask in 
reg_mask_init).

// Nils

On 2019-02-14 07:32, dean.long at oracle.com wrote:
> How about something like this:
>
> http://cr.openjdk.java.net/~dlong//8217909/webrev/
>
> dl
>
> On 2/13/19 5:05 AM, Roman Kennke wrote:
>> Apparently, this is a limitation of CodeSnippetRegClass, in 
>> formsopt.hpp:
>>
>>    void set_stack_version(bool flag) {
>>      assert(false, "User defined register classes are not allowed to
>> spill to the stack.");
>>    }
>>
>> I believe this means I cannot use this approach.
>>
>> I also don't feel like extending the AD syntax, parser, etc at this 
>> point.
>>
>> Do you have any other suggestions? Or should we go with my original 
>> patch?
>>
>> Roman
>>
>>> I started a version based on Dean's approach:
>>>
>>> http://cr.openjdk.java.net/~rkennke/JDK-8217909/webrev.01/
>>>
>>> Unfortunately it barks on ptr_reg class:
>>>
>>> assert fails
>>> /home/rkennke/src/openjdk/jdk-jdk/src/hotspot/share/adlc/formsopt.hpp
>>> 246: User defined register classes are not allowed to spill to the 
>>> stack.
>>>
>>> Interestingly, it doesn't do that when I only handle any_reg class.
>>> However, I am not sure what that means or how to fix it. Any hints?
>>>
>>> Thanks,
>>> Roman
>>>
>>>> There is a way to do it without cloning all the register class 
>>>> variants.
>>>> For the arm64 port we massaged the register masks in
>>>> Compile::pd_compiler2_init():
>>>>
>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/b756e7a2ec33/src/cpu/arm/vm/arm_64.ad#l638 
>>>>
>>>>
>>>>
>>>> It might be worth considering, as the number of variants grows.
>>>>
>>>> dl
>>>>
>>>> On 2/11/19 3:02 AM, Roman Kennke wrote:
>>>>> When running with compressed oops, the r12 register holds the 
>>>>> heapbase,
>>>>> and thus is not available to register allocation in C2. However, when
>>>>> *not* running with compressed oops, it is still not available and
>>>>> remains unused. It should be made available to register allocation in
>>>>> this case.
>>>>>
>>>>> This patch implements this by introducing with_r12 and no_r12 
>>>>> variants
>>>>> of basically all register classes, and add dynamic reg classes to 
>>>>> select
>>>>> one or the other, based on current settings. I needed to add 
>>>>> UseZGC in
>>>>> those flags because ZGC asserts to not get r12 in its barriers. 
>>>>> Not sure
>>>>> that this is necessary.
>>>>>
>>>>> Note that we might want to use the r12 register in Shenandoah 
>>>>> later to
>>>>> keep GC state. In this case, we'd need to add UseShenandoahGC or 
>>>>> such to
>>>>> the test. Do we want to abstract this whole check? Not sure that 
>>>>> this is
>>>>> possible/feasible to do in .ad though...
>>>>>
>>>>> Bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8217909
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8217909/webrev.00/
>>>>>
>>>>> Testing: tier1 no regressions locally, eyeball generated code, yes it
>>>>> does use r12 now.
>>>>>
>>>>> Can I please get reviews?
>>>>>
>>>>> Roman
>