RFR (S): 8024922: PPC64 (part 116): Extend adlc to generate fields into nodes.

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Tue Oct 1 16:25:42 PDT 2013

Hi Vladimir and Iris,

Thanks for the review, I pushed it but -- grrrr -- I forgot
to add you as reviewer!  I'm sorry, now there is no reviewer.
Can we fix this somehow?  Or can we roll back the change
as we did before? I think Iris, you helped with that,
could you have a look, please?
I thought jcheck would catch this?

I'm sorry for causing additional trouble,

-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Tuesday, October 01, 2013 8:27 PM
To: Lindenmaier, Goetz
Cc: ppc-aix-port-dev at openjdk.java.net; 'hotspot-dev developers'
Subject: Re: RFR (S): 8024922: PPC64 (part 116): Extend adlc to generate fields into nodes.

Thank you, Goetz

I think you described this enough for me to understand changes,
You can push these changes now. We don't have closed code for adlc so I think it is safe to push it without JPRT.


On 9/30/13 2:04 AM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>> So you did all this to schedule IC load as separate instruction. Right?
> More or less, yes.  But not only the IC load.  All nodes are broken down
> so that they represent a single assembler instruction.  (Except for real
> big ones as string compare, sync ...).  Mostly we use expand, but that's
> not always possible.  E.g. expanding DecodeN into add+shift early
> can cause problems, if the intermediate value which is neither oop nor
> narrow oop gets visible to GC.
>> Did you try to use 'expand %{ %}' for call node? We will generate
>> separate mach nodes in such case.
> I am using expand in several cases, but it does not work for call nodes.
> Call nodes are very special.
> Best regards,
>    Goetz.
> On 9/24/13 6:44 AM, Lindenmaier, Goetz wrote:
>> Hi Vladimir,
>>> First, when you say "constant pool" you really mean "constant section"
>>> in nmethod. Right? And you want to load constants from it.
>> Yes.
>>> I would like to see you use our implementation of constant loads but it
>>> is up to you if it does not affect our shared code.
>> I use your functionality to get the constant into the pool, and to get the
>> edge to the toc/constanttablebase.  But then we split the constant load
>> into two nodes, each encoding part of the offset into the pool.
>> That's when I need these fields.
>> Actually I fixed our nodes as far as possible to use that functionality
>> when I first pushed the port into the jdk7 directory.
>>> To have non-matching CallDynamicJavaDirect__2Node is interesting way to
>>> not add new nodes to machnode.hpp.
>>> How you keep a live mach node which loads IC (do you have users/edges
>>> pointing to it)? And how you scheduling it? And what about live range if
>>> it is separated from call site?
>> We add the toc/constanttablebase node to the call right after matching.
>> Unfortunately constanttablebase can not be used in call nodes, so we
>> Walk the graph and add it.  We misuse in(TypeFunc::ReturnAdr)
>> to point to toc.
>> After register allocation we run the lateExpand phase.
>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/e6d09cebf92d/ppc_patches/0115_8003850_opto-introduce_phase_lateExpand.patch
>> This splits the call node into the individual instructions needed,
>> which are two nodes for loading IC, two nodes for loading env ptr
>> to r11, two nodes for loading the address,
>> one node moving the address to the branch register, and
>> the real call.  This is then scheduled.  The scheduler replaces
>> Do_scheduling in output.cpp.
>> I would like to improve handling constanttablebase with the Call node.  There are
>> Two problems: you can only use constanttablebase with constant nodes.
>> But we need it with Call and storeCM, too.
>> The other problem is that you can not add req edges to Call nodes.  Therefore
>> we misuse ReturnAdr.  Do you have an idea how this can be achieved?
>> (This has nothing to do with this change.)
>> Best regards,
>>     Goetz
>> Thanks,
>> Vladimir
>> On 9/20/13 2:31 AM, Lindenmaier, Goetz wrote:
>>> Hi Vladimir,
>>> we use these fields in two situations.
>>> We use the constant pool. If the constant pool
>>> get's large, we need two instructions to encode the offset
>>> into the constant pool, basically looking like this:
>>>         TOC
>>>          |
>>>        exLoadConP_hiNode (Adds upper bits of offset to TOC)
>>>          |
>>>        exLoadConP_loNode (Load with immediate offset)
>>> When we emit *_hiNode, we get the offset into the constant pool.
>>> We can only encode the upper bits, thus we remember the lower bits in
>>> the field generated by ins_field_const_toc_offset(int).
>>> When we emit the *_loNode, we need access to the *_hiNode to retrieve the
>>> remaining bits of the offset. For this, we remember the *_hiNode in field
>>> ins_field_const_toc_offset_hi_node(exLoadConP_hiNode*) when we
>>> generate the two nodes.
>>> To remember data we need to generate relocations we use fields
>>>      ins_field_load_ic_hi_node(exLoadConL_hiNode*);
>>>      ins_field_load_ic_node(exLoadConLNode*);
>>>      ins_field_cbuf_insts_offset(int);
>>> We load the inline cache from the constant pool. For this we
>>> separate a constant node from the dynamic call node.
>>> When we generate the relocation for the inline cache of the call node,
>>> we use _load_ic_[hi_]node fields to find the node loading the ic cache.
>>> For the virtual_call_Relocation we must know the location of the node
>>> loading the constant, to find the constant in the constant pool.
>>> The exLoadConL[_hi]Node remembers it's offset in the code cache
>>> in field _cbuf_insts_offset.
>>> Constants were a performance problem in our port until we used the
>>> constant pool (we had one before it was added to HotSpot, but
>>> after the instruction section). Further we want separate nodes
>>> for each assembler instruction - especially for frequently used
>>> nodes as constants and calls - to allow better scheduling.
>>> I'm happy to supply more details, or to implement this differently,
>>> as long as I can keep the separate nodes.  Maybe I have to change
>>> the relocation?
>>> Best regards,
>>>      Goetz.
>>> -----Original Message-----
>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>> Sent: Donnerstag, 19. September 2013 19:25
>>> To: Lindenmaier, Goetz
>>> Cc: ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
>>> Subject: Re: RFR (S): 8024922: PPC64 (part 116): Extend adlc to generate fields into nodes.
>>> Could you explain why you need _load_ic_*_node fields
>>> CallDynamicJavaDirect mach node? I am trying to understand if there is
>>> an other, already existing, way to do that. I am fine with these changes
>>> but I need to know why.
>>> Thanks,
>>> Vladimir
>>> On 9/19/13 8:44 AM, Lindenmaier, Goetz wrote:
>>>> Hi,
>>>> We extended adlc by a feature that allows to specify fields of
>>>> MachNodes.
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8024922-adlcFields/
>>>> This is implemented according to the ins_xxx() functionality that
>>>> allows to specify functions returning constants.  If you specify
>>>> an ins_field_xxx(tp) in an instruct specification, a field _xxx
>>>> with type tp is added to the node.
>>>> You can see a usage of this feature in the ppc.ad file:
>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/e6d09cebf92d/src/cpu/ppc/vm/ppc.ad
>>>> E.g., on line 12928 you find the specification of _load_ic_hi_node:
>>>>         12928 instruct CallDynamicJavaDirect__2(method meth) %{
>>>>         12924   match(CallDynamicJava); // To get all the data fields we
>>>> need ...
>>>>         12925   effect(USE meth);
>>>>         12926   predicate(false);       // ... but never match.
>>>>         12927
>>>>         12928   ins_field_load_ic_hi_node(exLoadConL_hiNode*);
>>>> which is used on line 5098:
>>>>          5098     {
>>>>          5099       CallDynamicJavaDirect__2Node *m1 =
>>>> (CallDynamicJavaDirect__2Node *)call;
>>>>          5100       m1->_load_ic_hi_node = exLoadConLNodes_IC._large_hi;
>>>>          5101       m1->_load_ic_node    = exLoadConLNodes_IC._small;
>>>>          5102     }
>>>> As with other ins_ attributes, a general declaration of the attribute is
>>>> needed, see
>>>> line 6565:
>>>>          6565 ins_attrib ins_field_load_ic_hi_node(0);
>>>> In adlc, we just had to change the output of nodes. Parsing of the ad file
>>>> is not affected.
>>>> This change only affects adlc. There should be no effects on the Oracle
>>>> platforms, except if a closed platform happens to specify an attribute
>>>> with the name prefix ins_field_.
>>>> Please review and test this change.
>>>> Best regards,
>>>>       Goetz.

More information about the hotspot-dev mailing list