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

Christian Thalinger christian.thalinger at oracle.com
Mon Mar 25 15:28:32 PDT 2013 

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