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

[dean.long at oracle.com]

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
>>