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

[Roman Kennke]

Ok, this should be equivalent to the current situation, including your
debugging code that verifies that the register classes are indeed the same:

http://cr.openjdk.java.net/~rkennke/JDK-8217909/webrev.02/

Those verifications will disappear from the final checks, because:
1. I'm going to modify the stuff, based on the r12 issue
2. I don't want to drag around the old register classes just for this
(sortof defeats the purpose..)

I wanted to post this as a sortof 'milestone' so that it's easier to see
later that the register classes are indeed the same, and what exactly I
did change from now on.

I think we can generate the STACK_OR_* classes by adlc without much
complications, but as you say, better to do it in separate RFE.

Thanks, Dean, for helping!
Roman

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20190218/887e3938/signature.asc>