RFR (S): 8003853: specify offset of IC load in java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load in java_to_interp stub.]
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Tue Apr 16 07:22:34 PDT 2013
Hi Dean,
I left the runtime call in the ad file, as it's got nothing to do with
CompiledStaticCalls that call Java.
I thought this is a better separation of concerns.
Anyways, you would have to pass in _method and
_optimized_virtual, which are fields of the machnode.
Best regards,
Goetz.
-----Original Message-----
From: hotspot-compiler-dev-bounces at openjdk.java.net [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Dean Long
Sent: Dienstag, 16. April 2013 07:20
To: hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR (S): 8003853: specify offset of IC load in java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load in java_to_interp stub.]
Also, is moving the call into CompiledStaticCall::emit_call() really
needed for PPC? I'd prefer to move the
stub emitter instead (sample sparc code):
if ( !_method ) {
emit_call_reloc(cbuf, $meth$$method, relocInfo::runtime_call_type);
} else if (_optimized_virtual) {
emit_call_reloc(cbuf, $meth$$method,
relocInfo::opt_virtual_call_type);
} else {
emit_call_reloc(cbuf, $meth$$method, relocInfo::static_call_type);
}
if( _method ) { // Emit stub for static call
CompiledStaticCall::emit_java_to_interp(cbuf);<---
}
dl
On 4/15/2013 9:54 PM, Dean Long wrote:
> sparc.ad:
>
> 2538 CompiledStaticCall::emit_call(cbuf,
> 2539 (int) ($meth$$method -
> ((intptr_t) cbuf.insts_end()) - 4),
> 2540 rtype);
>
> I think this should be:
>
> CompiledStaticCall::emit_call(cbuf,$meth$$method, rtype);
>
> Computing the PC-relative offset probably only applies to x86.
>
> dl
>
> On 4/12/2013 2:03 PM, Christian Thalinger wrote:
>> On Apr 11, 2013, at 2:46 PM, "Lindenmaier, Goetz"
>> <goetz.lindenmaier at sap.com> wrote:
>>
>>> Hi all,
>>>
>>> I prepared the webrev for the changes discussed below:
>>> http://cr.openjdk.java.net/~goetz/webrevs/8003853/
>>>
>>> To fix the issues we have with the PPC port with finding ICs, this
>>> change
>>> cleans up the code handling compiled static Java calls:
>>> - Methods using native instructions are moved from compiledIC.cpp
>>> to compiledIC_<cpu>.cpp.
>>> - Methods emitting the call patched by CompiledStaticCall are
>>> moved from the ad file to this class.
>>> As a side effect this reduces redundancies in x86_64.ad and
>>> x86_32.ad.
>>> - We get rid of extern declarations in output.cpp.
>> I like this. Two comments:
>>
>> 1) Does it make sense to have emit_to_interp_stub when it's only used
>> in one place?
>>
>> 2) Before we can push this someone has to do the same thing for ARM
>> and PPC. Any volunteers from the embedded team?
>>
>> -- Chris
>>
>>> Now all code concerning CompiledStaticCalls is collected in one class.
>>> The PPC port needs not change shared code any more to implement them.
>>> I also got around to move code to MacroAssembler, as only a simple call
>>> was needed on x86 and sparc.
>>>
>>> Probably this is (M) now, no more (S).
>>>
>>> Please review and send comments. I'll be happy to adapt to them!
>>>
>>> Best regards,
>>> Goetz Lindenmaier
>>>
>>> -----Original Message-----
>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>> Sent: Thursday, April 11, 2013 4:50 AM
>>> To: Lindenmaier, Goetz
>>> Cc: 'hotspot-compiler-dev at openjdk.java.net'
>>> Subject: Re: RFR (S): 8003853: specify offset of IC load in
>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load in
>>> java_to_interp stub.]
>>>
>>>
>>> On Apr 9, 2013, at 7:35 AM, "Lindenmaier, Goetz"
>>> <goetz.lindenmaier at sap.com> wrote:
>>>
>>>> Hi Chris,
>>>>
>>>> I'm now working on this issue. Sorry for the delay...
>>>>
>>>> What do you think about moving
>>>> void emit_java_to_interp(CodeBuffer& cbuf);
>>>> and
>>>> void size_java_to_interp();
>>>> to compiledIC.hpp and the new platform file, too?
>>> I like that.
>>>
>>>> This would also allow to overcome the ugly
>>>> extern uint size_java_to_interp();
>>>> in output.cpp, as it could be replaced by
>>>> CompiledStaticCall::size_java_to_interp().
>>> Great.
>>>
>>>> I also would like to name this
>>>> CompiledStaticCall::emit_to_interp_stub()
>>>> CompiledStaticCall::to_interp_stub_size()
>>> I'm okay with that. Not sure if others have objections.
>>>
>>>> I could also extract code from enc_class Java_Static_Call
>>>> to form a routine
>>>> CompiledStaticCall::emit_call(CodeBuffer *cbuf, intptr_t
>>>> entry_point, relocInfo::relocType rtype) {
>>>> emit_call_reloc(cbuf, entry_point, rtype)
>>>> if (rtype != relocInfo::runtime_call_type) {
>>>> emit_java_to_interp(cbuf);
>>>> }
>>>> }
>>>> With this, also
>>>> extern uint reloc_java_to_interp();
>>>> from output.cpp could be moved into this class.
>>> On SPARC only or the other architectures as well?
>>>
>>>> Then handling of CompiledStaticCalls would be completely
>>>> implemented in this
>>>> one class.
>>> That's tempting.
>>>
>>>> (I would have to move emit_call_reloc to MacroAssembler.)
>>> Could you send a webrev with these changes?
>>>
>>> -- Chris
>>>
>>>> Best regards,
>>>> Goetz.
>>>>
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>> Sent: Montag, 25. März 2013 23:29
>>>> To: Lindenmaier, Goetz
>>>> Cc: 'hotspot-compiler-dev at openjdk.java.net'
>>>> Subject: Re: RFR (S): 8003853: specify offset of IC load in
>>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load in
>>>> java_to_interp stub.]
>>>>
>>>>
>>>> On Mar 4, 2013, at 8:34 AM, "Lindenmaier, Goetz"
>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>
>>>>> Hi Chris,
>>>>>
>>>>> I would like to take up this issue again.
>>>>> I'm about to put our changes on the jdk8 branch, and it would be
>>>>> convenient
>>>>> to have a final solution for this.
>>>>>
>>>>> Should I prepare a webrev with a different solution adding a new file
>>>>> for this? What should go into that file?
>>>> Sorry for answering that late.
>>>>
>>>> Yes, to see a patch for that would be good. We need to put all
>>>> platform dependent Compiled* methods in that new file. Platform
>>>> dependent means every method that uses a native* method from
>>>> nativeInst_<arch>:
>>>>
>>>> CompiledIC::cleanup_call_site
>>>> CompiledIC::is_icholder_call_site
>>>> CompiledIC::CompiledIC
>>>>
>>>> CompiledStaticCall::set_to_interpreted
>>>> CompiledStaticCall::set_stub_to_clean
>>>> CompiledStaticCall::verify
>>>>
>>>> -- Chris
>>>>
>>>>> Best regards,
>>>>> Goetz.
>>>>>
>>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: goetz.lindenmaier at sap.com
>>>>> Sent: Dienstag, 18. Dezember 2012 10:42
>>>>> To: 'Christian Thalinger'
>>>>> Subject: RE: RFR (S): 8003853: specify offset of IC load in
>>>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load
>>>>> in java_to_interp stub.]
>>>>>
>>>>> Hi Chris,
>>>>>
>>>>> What would you put into that file? The offset, or the class
>>>>> CompiledStaticCall?
>>>>> You could also put it into nativeInst_<arch> to avoid adding
>>>>> another file.
>>>>>
>>>>> But basically I think the offset is the best solution, because
>>>>> it's very small
>>>>> and straight forward, and the offset is set where the code is
>>>>> generated
>>>>> that defines the offset.
>>>>>
>>>>> Only the #ifndef COMPILER2 is ugly, but here I think the solution
>>>>> is to
>>>>> guard the whole class CompiledStaticCall with #ifdef COMPILER2.
>>>>>
>>>>> But I'm happy with any placement of the code.
>>>>>
>>>>> Best regards,
>>>>> Goetz.
>>>>>
>>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>> Sent: Montag, 17. Dezember 2012 21:34
>>>>> To: Lindenmaier, Goetz
>>>>> Cc: 'hotspot-compiler-dev at openjdk.java.net'
>>>>> Subject: Re: RFR (S): 8003853: specify offset of IC load in
>>>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load
>>>>> in java_to_interp stub.]
>>>>>
>>>>>
>>>>> On Nov 29, 2012, at 2:10 PM, "Lindenmaier, Goetz"
>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>
>>>>>> Hi Chris,
>>>>>>
>>>>>> Loading the constant takes 5 instructions, the other way only three,
>>>>>> in the code only one, if the constant pool base is in a register.
>>>>>> The 5 instructions are hard to patch atomically. The constant pool
>>>>>> is just a 64 bit store.
>>>>>>
>>>>>>> Sorry for being so persistent on this :-)
>>>>>> That's ok, I already found several places in our VM where it was
>>>>>> easy to
>>>>>> change our code to get along without the shared changes we did in
>>>>>> first place. But I can't redesign everything, and I can not risk
>>>>>> to break
>>>>>> anything in our product.
>>>>>>
>>>>>> If you feel like doing so, you can have a look at
>>>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk7u/hotspot/rev/600ce596eea9
>>>>>>
>>>>>> which contains our bytecode interpreter profiling implementation.
>>>>>> If you think this is currently of use in the main branch, I would
>>>>>> lift it to HS25
>>>>>> (the permgen changes might cause trouble) and contribute it to hsx.
>>>>> Sorry for the long delay.
>>>>>
>>>>> Thinking about this problem again it seems that the correct
>>>>> solution would be to have a compiledIC_<arch>.cpp file.
>>>>>
>>>>> The CompiledIC instruction layout is platform dependent; it just
>>>>> happens to be the same for all architectures we have right now
>>>>> (well, except PPC) because we made it to be the same. Actually on
>>>>> SPARC we could also use a load from the constant table which would
>>>>> be simpler.
>>>>>
>>>>> Having a platform dependent file for this code feels right. What
>>>>> do you think?
>>>>>
>>>>> -- Chris
>>>>>
>>>>>> Best regards,
>>>>>> Goetz.
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>>> Sent: Thursday, November 29, 2012 7:53 PM
>>>>>> To: Lindenmaier, Goetz
>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net
>>>>>> Subject: Re: RFR (S): 8003853: specify offset of IC load in
>>>>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load
>>>>>> in java_to_interp stub.]
>>>>>>
>>>>>>
>>>>>> On Nov 29, 2012, at 12:27 AM, "Lindenmaier, Goetz"
>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>
>>>>>>> Hi Chris,
>>>>>>>
>>>>>>>> Why can't you point the relocation to the method load
>>>>>>>> instruction? It doesn't seem right to do that in shared code.
>>>>>>> Because the relocation is used
>>>>>>> 1.) to find the beginning of the stub to patch the call to go there
>>>>>>> 2.) to find the load to patch the IC.
>>>>>> Got it.
>>>>>>
>>>>>>> Just have a look at CompiledStaticCall::set_to_interpreted(),
>>>>>>> the variable 'stub' is used twice.
>>>>>> Sorry for being so persistent on this :-) What is the reason you
>>>>>> have to load the constant from the constant table? On other
>>>>>> architectures we materialize the constant.
>>>>>>
>>>>>> -- Chris
>>>>>>
>>>>>>> Best regards,
>>>>>>> Goetz.
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>>>> Sent: Donnerstag, 29. November 2012 01:35
>>>>>>> To: Lindenmaier, Goetz
>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net
>>>>>>> Subject: Re: RFR (S): 8003853: specify offset of IC load in
>>>>>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC load
>>>>>>> in java_to_interp stub.]
>>>>>>>
>>>>>>>
>>>>>>> On Nov 27, 2012, at 4:24 AM, "Lindenmaier, Goetz"
>>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>>
>>>>>>>> Hi Chris,
>>>>>>>>
>>>>>>>>> Why do you need all these shared code changes when you have
>>>>>>>>> this in emit_java_to_interp_stub?
>>>>>>>> Shared code (class CompiledStaticCall) wants to patch the
>>>>>>>> inline cache
>>>>>>>> in the stub which is emitted in platform code.
>>>>>>>> So the platform code should specify how the shared code can
>>>>>>>> find the inline cache
>>>>>>>> in the stub, which it does through the constant I introduced.
>>>>>>>>
>>>>>>>>> Do you need two relocations? I'm confused.
>>>>>>>> No, three ;)
>>>>>>>> The relocation you mention is used to find the stub given the
>>>>>>>> call.
>>>>>>>> The other relocations are needed to find the inline cache /
>>>>>>>> call target
>>>>>>>> once the stub is found (see
>>>>>>>> CompiledStaticCall::set_to_interpreted()).
>>>>>>>> It's just the same on x86_64:
>>>>>>>>
>>>>>>>> // static stub relocation stores the instruction address of the
>>>>>>>> call
>>>>>>>> __ relocate(static_stub_Relocation::spec(mark), RELOC_IMM64);
>>>>>>>> // static stub relocation also tags the methodOop in the
>>>>>>>> code-stream.
>>>>>>>> __ movoop(rbx, (jobject) NULL); // method is zapped till fixup
>>>>>>>> time
>>>>>>>> // This is recognized as unresolved by relocs/nativeinst/ic code
>>>>>>>> __ jump(RuntimeAddress(__ pc()));
>>>>>>>>
>>>>>>>> On PPC, we have to load the base of the constant table after
>>>>>>>> generating the
>>>>>>>> static_stub_relocation and before emitting the ppc equivalent
>>>>>>>> for moveoop(). This offset is
>>>>>>>> exactly what we store in the
>>>>>>>> CompiledStaticCall::comp_to_int_load_offset constant.
>>>>>>> Why can't you point the relocation to the method load
>>>>>>> instruction? It doesn't seem right to do that in shared code.
>>>>>>>
>>>>>>> -- Chris
>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Goetz.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>>>>> Sent: Dienstag, 27. November 2012 01:53
>>>>>>>> To: Lindenmaier, Goetz
>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net
>>>>>>>> Subject: Re: RFR (S): 8003853: specify offset of IC load in
>>>>>>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC
>>>>>>>> load in java_to_interp stub.]
>>>>>>>>
>>>>>>>>
>>>>>>>> On Nov 22, 2012, at 8:51 AM, "Lindenmaier, Goetz"
>>>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>>>
>>>>>>>>> Hi Chris,
>>>>>>>>>
>>>>>>>>> We put it into the ad file because it describes an offset into
>>>>>>>>> the stub that is
>>>>>>>>> generated from the ad file by emit_java_to_interp(CodeBuffer&
>>>>>>>>> cbuf),
>>>>>>>>> used by ir node CallStaticJavaDirect. Maybe I should not
>>>>>>>>> call it stub, but it's
>>>>>>>>> generated into the stubs section of the code buffer.
>>>>>>>>> If that stub generator is moved out of the ad file to shared
>>>>>>>>> compiler code, one should
>>>>>>>>> move the field with it, but for the current implementation I
>>>>>>>>> think that's fine.
>>>>>>>>>
>>>>>>>>> Does C1 generate the same stub? Or is the CompiledStaticCall a
>>>>>>>>> C2 feature?
>>>>>>>>> If so, one could protect the whole thing by #ifdef COMPILER2.
>>>>>>>> Why do you need all these shared code changes when you have
>>>>>>>> this in emit_java_to_interp_stub?
>>>>>>>>
>>>>>>>> // Create a static stub relocation which relates this stub
>>>>>>>> // with the call instruction at insts_call_instruction_offset
>>>>>>>> in the
>>>>>>>> // instructions code-section.
>>>>>>>> __ relocate(static_stub_Relocation::spec(__
>>>>>>>> code()->insts()->start() + insts_relocation_offset));
>>>>>>>>
>>>>>>>> Do you need two relocations? I'm confused.
>>>>>>>>
>>>>>>>> -- Chris
>>>>>>>>
>>>>>>>>> Thanks for all the bugids!
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Goetz.
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>>>>>> Sent: Mittwoch, 21. November 2012 19:53
>>>>>>>>> To: Lindenmaier, Goetz
>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net
>>>>>>>>> Subject: RFR (S): 8003853: specify offset of IC load in
>>>>>>>>> java_to_interp stub [Was: Re: RFR (S): Specify offset of IC
>>>>>>>>> load in java_to_interp stub.]
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Nov 21, 2012, at 12:15 AM, "Lindenmaier, Goetz"
>>>>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Chris,
>>>>>>>>>>
>>>>>>>>>> yes, there is no C1 on ppc.
>>>>>>>>>> We do tiered compilation with a stripped C2.
>>>>>>>>> Right. But some other architecture (or in the future) might
>>>>>>>>> need this for C1 as well. We should find a better place for
>>>>>>>>> the value than the ad file. The obvious place would be
>>>>>>>>> compiledIC_<arch>.hpp but we don't have that file, yet.
>>>>>>>>>
>>>>>>>>> I filed:
>>>>>>>>>
>>>>>>>>> 8003853: specify offset of IC load in java_to_interp stub
>>>>>>>>>
>>>>>>>>> -- Chris
>>>>>>>>>
>>>>>>>>>> Best regards,
>>>>>>>>>> Goetz
>>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Christian Thalinger
>>>>>>>>>> [mailto:christian.thalinger at oracle.com]
>>>>>>>>>> Sent: Dienstag, 20. November 2012 22:19
>>>>>>>>>> To: Lindenmaier, Goetz
>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net
>>>>>>>>>> Subject: Re: RFR (S): Specify offset of IC load in
>>>>>>>>>> java_to_interp stub.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Nov 20, 2012, at 3:08 AM, "Lindenmaier, Goetz"
>>>>>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> The class CompiledStaticCall must know the location of the
>>>>>>>>>>> IC load in the java_to_interp stub
>>>>>>>>>>> to update the IC with NativeMovConstReg. The current
>>>>>>>>>>> implementation assumes that
>>>>>>>>>>> the load is always the first instruction in the stub. This
>>>>>>>>>>> is an artificial restriction. E.g.,
>>>>>>>>>>> it might be useful to load the constant pool address
>>>>>>>>>>> (MachConstantBase) before the
>>>>>>>>>>> IC load (as we do on PPC).
>>>>>>>>>>>
>>>>>>>>>>> This change adds a constant specifying an offset from the
>>>>>>>>>>> beginning of the stub to
>>>>>>>>>>> the IC load. The offset is platform specific and 0 on sparc
>>>>>>>>>>> and x86.
>>>>>>>>>>>
>>>>>>>>>>> You can find the change here:
>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/webrev-IC_offset/
>>>>>>>>>> That should be fixed indeed.
>>>>>>>>>>
>>>>>>>>>> +#ifndef COMPILER2
>>>>>>>>>> +const int CompiledStaticCall::comp_to_int_load_offset = 0;
>>>>>>>>>>
>>>>>>>>>> There is no C1 for PPC (sorry, I didn't check)?
>>>>>>>>>>
>>>>>>>>>> -- Chris
>>>>>>>>>>
>>>>>>>>>>> or in our ppc port:
>>>>>>>>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk7u/hotspot/rev/c6f9c897ea33
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thank you and best regards,
>>>>>>>>>>> Goetz
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>
More information about the hotspot-compiler-dev
mailing list