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,
Goetz.
-----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.
Thanks,
Vladimir
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