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

[dean.long at oracle.com]

On 2/18/19, 12:23 PM, Roman Kennke wrote:
> Somehow I missed it when I looked at it back when you posted it. Have 
> you updated it in place in the meantime? 
Not that I recall :-)
> Anyhow, this looks promising, I will try to take it from there. Thanks!!
>

Great, thanks!

dl

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