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

Iris Clark iris.clark at oracle.com
Wed Oct 2 14:23:41 PDT 2013

Hi, Goetz.

I've rolled back the problematic changeset.  You should be set.

I'm not quite sure why jcheck did not detect this problem and am investigating in a separate thread.


-----Original Message-----
From: Lindenmaier, Goetz [mailto:goetz.lindenmaier at sap.com] 
Sent: Tuesday, October 01, 2013 4:26 PM
To: Vladimir Kozlov; Iris Clark (iris.clark at oracle.com)
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.

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/e6d09cebf92
>> d/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/e6d09cebf
>>>> 92d/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