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]

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.

Just have a look at CompiledStaticCall::set_to_interpreted(),
the variable 'stub' is used twice.  

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