RFR (M): 8030863: PPC64: (part 220): ConstantTableBase for calls between args and jvms

[Vladimir Kozlov]

Thank you, now I understand, I think (at least why you can't use req()-1 
edge).

So the problem exist only for CallDynamicJava but you decided do set new 
const base edge for all call nodes (except leaf calls which do not have 
debug info). Right?

How it worked before? Expand() was called before all edges are added at 
the end of Matcher::match_tree().

Why you need to move clone_jvms() from Call node to SafePoint node 
(changes in node.cpp and callnode.hpp)? Only Call nodes are affected.
Add comment that we also need to clone jvms when call node needs to add 
an edge to MachConstantBaseNode during matching which will require jvms 
adjustment.

I don't like matcher_modifies_jvms name (it is too strong/general 
statement when it only shifts it). Can it be calls_need_constant_base? 
It affects almost all calls (even for Leaf calls it will be after 
arguments), it is more specific and can be used for other purposes.

Why you need to declare _matcher_modifies_jvms in FrameForm (changes in 
formsopt.*)?

JVMState::adapt_position(int delta). I would suggest to use a loop as in 
other places (yes, recursion is not deep, as you pointed, but it could 
be called with deep stack already):

   for (JVMState* jvms = this; jvms != NULL; jvms = jvms->caller()) {

Thanks,
Vladimir

On 12/24/13 3:25 PM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> yes, I meant postalloc expand.
> I was aware of the problem with jvms and build_oop_map, i.e.,
> that you spoil build_oop_map if you add an edge and don't somehow
> tell build_oop_map where to find the derived base pairs after that.
>
> But as I replace the node by an other one without the edge during
> postalloc expand, I figured I'd be safe.  But, as regalloc added the
> derived base edges behind the one I added, req()-1 pointed to the
> last base edge, and I would have removed the wrong edge.
>
> For the same reason, regalloc associated the register mask I
> supplied for the ConstTableBase for the node at req()-1, again
> the base edge of a derived/base pair.
>
> So I need a fixed position, which is now between the args
> and the edges for jvms.
>
> Best regards,
>    Goetz.
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Monday, December 23, 2013 9:01 PM
> To: Lindenmaier, Goetz; 'hotspot-dev at openjdk.java.net'; 'ppc-aix-port-dev at openjdk.java.net'
> Subject: Re: RFR (M): 8030863: PPC64: (part 220): ConstantTableBase for calls between args and jvms
>
> Goetz,
>
> Can you explain the problem in details? When you say 'during expand' do
> you mean 'during postalloc_expand'? Because normal expand happens during
> matching. And I did not get how it is related to derived/base pairs.
>
> Thanks,
> Vladimir
>
> On 12/20/13 7:20 AM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>> The change "8028580: PPC64 (part 114/120): Support for Call nodes with constants. "
>> adds MachConstantBase node for Calls at req()-1 during
>> expand. Register allocation adds the derived/base pairs and then
>> uses the wrong register maps for allocation, because the MachConstantBase
>> node is no more at req()-1
>>
>> Now add the edge in the matcher when the node is complete. Add it
>> after parameters and before jvms. Adapt jvms offsets. Also assure
>> jvms is always cloned, else the offset gets wrong.
>>
>> So far, $constanttablebase is only used in Calls on PPC, so this has no effect on
>> other platforms.
>>
>> Please review and test this change.
>> http://cr.openjdk.java.net/~goetz/webrevs/8030863-0-call/
>>
>> Best regards,
>>     Goetz.
>>