[9] RFR(S): 8075798: Allow ADLC register class to depend on runtime conditions also for cisc-spillable classes

[Dean Long]

On 3/27/2015 2:51 AM, Zoltán Majó wrote:
> Hi Dean,
>
>
> On 03/26/2015 10:12 PM, Dean Long wrote:
>> Hi Zoltan.  Could you explain what "cisc-spillable" means, and what 
>> test causes the output_c.cpp assert, an existing test or a new test 
>> that you will add?
>
> I don't fully understand the details behind cisc-spillable register 
> classes either, but here is what I think they are good for.
>
> Most .ad files define a cisc-spilling operand. For example, on x86_64 
> the cisc-spilling operand is indOffset32:
>
>   // Optional: name the operand used by cisc-spilling to access
>   // [stack_pointer + offset]
>   cisc_spilling_operand_name(indOffset32);
>
> The operand indOffset32 is defined the .ad file and represents an 
> indirect memory access with an offset.
>
> If a cisc-spilling operand is defined, ADLC checks for each 
> instruction if it can spill to an "alternate" (see 
> InstructForm::cisc_spills_to in formssel.cpp).
>
> For example, on x86_64 the instruction 'popCountI' can cisc-spill to 
> 'popCountI_mem'. (You can enable tracing of cisc-spilling by specifing 
> ADLCFLAGS += -s in adlc.make.) What that means, I think, is that a 
> popCountI instruction
>
> instruct popCountI(rRegI dst, rRegI src, rFlagsReg cr)
>
> can be "replaced" by the instruction
>
> instruct popCountI_mem(rRegI dst, memory mem, rFlagsReg cr)
>
> if 'rRegI src' is known to have been spilled onto the stack when the 
> popCountI would be emitted. This makes it possible to access that 
> stack location without loading it back to a register and then storing 
> it back again. I can imagine some scenarios when this might be 
> beneficial.
>
> I have not traced this actually until the end, but I imagine it might 
> work like this. Please correct me if I'm wrong.
>

It sounds good to me.  Thanks!

> The presence of cisc-spillable operands can impose problems for ADLC 
> code that depends on runtime conditions. For example, if we have a 
> register class actual_dflt_reg that depends on a runtime condition
>
> reg_class actual_dflt_reg %{
>   if (VM_Version::has_vfp3_32()) {
>     return DFLT_REG_mask();
>   } else {
>     return DFLT_LOW_REG_mask();
>   }
> %}
>
> that register class can be used only in operands that are referred to 
> by instructions that have no cisc-spilled alternative.
>
> There is no test case where this problem would appear right now, but 
> for my upcoming changes for 8068945 I need cisc-spillable register 
> classes that depend on runtime conditions.
>

OK.

dl

>> I didn't see any problems with the changes, though I must admit 
>> looking at ADLC code makes my head hurt!
>
> Thanks a lot for the review!
>
> Best regards,
>
>
> Zoltan
>
>>
>> dl
>>
>> On 3/26/2015 3:00 AM, Zoltán Majó wrote:
>>> Hi,
>>>
>>>
>>> please review the following patch.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8075798
>>>
>>>
>>> Problem: In .ad files, it is difficult to have register classes that 
>>> depend on runtime conditions. JDK-7090968 partially solves the 
>>> problem, but it does not work in cases when not only a register 
>>> mask, but also a
>>> "stack mask" is generated for a register class (i.e., the register 
>>> class is "cisc-spillable"). (If a stack mask is needed for classes 
>>> dependant on runtime conditions, the assert on line 155 in 
>>> src/share/vm/adlc/output_c.cpp fails.)
>>>
>>> The goal of this enhancement is to extend ADLC parsing to allow 
>>> cisc-spillable register classes to depend on runtime conditions.
>>>
>>>
>>> Solution: Introduce a new register class, reg_class_dynamic. The new 
>>> register class is used with the following syntax:
>>>
>>> reg_class_dynamic actual_dflt_reg(dflt_reg, low_reg, %{ 
>>> VM_Version::has_vfp3_32() }% );
>>>
>>> where dflt_reg and low_reg are two existing "static" register 
>>> classes and actual_dflt_reg is the newly defined register class.
>>>
>>> The functionality of the actual_dflt_reg defined with 
>>> reg_class_dynamic is equivalent to the existing syntax
>>>
>>> reg_class actual_dflt_reg %{
>>>     if (VM_Version::has_vfp3_32()) {
>>>         return DFLT_REG_mask();
>>>     } else {
>>>         return DFLT_LOW_REG_mask();
>>>     }
>>> }
>>>
>>> The difference between the two register class declarations is that 
>>> the declaration with reg_class_dynamic allows the declared register 
>>> class to be "cisc-spillable", whereas a declaration with reg_class 
>>> does not. The code attached to a reg_class can be, however, more 
>>> elaborate than a simple if-then-else allowed with reg_class_dynamic.
>>>
>>>
>>> Webrev: http://cr.openjdk.java.net/~zmajo/8075798/webrev.00/
>>>
>>>
>>> Testing:
>>> - JPRT testing on all supported platforms
>>> - manual testing on x86_64 with a .ad file that includes a reg_class 
>>> definition with a code snippet as well as a reg_class_dynamic 
>>> definition.
>>>
>>>
>>> Thank you and best regards,
>>>
>>>
>>> Zoltan
>>>
>>
>