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

[Roman Kennke]

Somehow I missed it when I looked at it back when you posted it. Have 
you updated it in place in the meantime? Anyhow, this looks promising, I 
will try to take it from there. Thanks!!

Roman


> You don't see it in the webrev I posted?
> 
> http://cr.openjdk.java.net/~dlong//8217909/webrev/
> 
> dl
> 
> On 2/18/19 11:14 AM, Roman Kennke wrote:
>> No I haven't. Have you posted this somewhere and I missed it? :-)
>>
>> I will try it out. Thanks!
>>
>> Roman
>>
>>
>>> We can do that in a followup RFE.  For now, did you see how I did it 
>>> for PTR only?
>>>
>>> _STACK_OR_PTR_REG_mask = _PTR_REG_mask;
>>> _STACK_OR_PTR_REG_mask.OR(STACK_OR_STACK_SLOTS_mask());
>>>
>>>
>>> dl
>>>
>>> On 2/18/19 3:43 AM, Roman Kennke wrote:
>>>> I don't know how to do that, though. How should I proceed? The issue is
>>>> a nice-to-have for me, but not high-enough priority to dive into the
>>>> guts of adlc.
>>>>
>>>> Roman
>>>>
>>>>
>>>>> Yes it does, but in this case, only for the PTR class, right? However,
>>>>> it would be nice if
>>>>> adlc would generate it for us.
>>>>>
>>>>> dl
>>>>>
>>>>> On 2/14/19 2:47 AM, Roman Kennke wrote:
>>>>>> It is not so easy.
>>>>>>
>>>>>> We'd also need to generate STACK_OR_$NAME_mask() accessors for 
>>>>>> spillable
>>>>>> registers. See RegClass::build_register_masks().
>>>>>>
>>>>>> Roman
>>>>>>
>>>>>>> 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
>>>
>