RFR (S): 8024922: PPC64 (part 116): Extend adlc to generate fields into nodes.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Wed Oct 2 14:37:37 PDT 2013
Hi Iris,
thank you for the help! I'll push the change once more.
Sorry for the trouble,
Goetz.
-----Original Message-----
From: Iris Clark [mailto:iris.clark at oracle.com]
Sent: Wednesday, October 02, 2013 11:24 PM
To: Lindenmaier, Goetz; Vladimir Kozlov
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, 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.
Thanks,
iris
-----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,
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/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