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

[Vladimir Ivanov]

>> Do you need anything special for 32-bit x86 port in x86_32.ad?
> 
> Not that I know of. r12 and the compressed oops stuff is only ever
> present in 64bit. We might want to check how it's done on other 64bit
> platforms though..

Since you add reg_mask_init() usage into c2_init_x86.cpp, won't its 
absence cause problems at linkage there?

Best regards,
Vladimir Ivanov

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