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.
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