RFR(M): 8037821: Account for trampoline stubs when estimating code buffer sizes

[Schmidt, Lutz]

Yes, Vladimir, 
stub size calculation needs to be fixed in 8u as well. We discovered that deficiency a bit too late...
Thanks and sorry for the add'l effort!
Lutz

-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Montag, 24. März 2014 17:38
To: Schmidt, Lutz; hotspot-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
Subject: Re: RFR(M): 8037821: Account for trampoline stubs when estimating code buffer sizes

Lutz,

Do I need to backport it into 8u ppc-aix/stage repo?

Thanks,
Vladimir

On 3/24/14 9:07 AM, Schmidt, Lutz wrote:
> Hi Vladimir,
> thanks for the (positive) review! I'll stand by...
> Cheers, Lutz
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Montag, 24. März 2014 16:56
> To: Schmidt, Lutz; hotspot-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> Subject: Re: RFR(M): 8037821: Account for trampoline stubs when estimating code buffer sizes
>
> Looks good to me.
>
> Let me do closed part. I will push it after closed part is reviewed/approved.
>
> Thanks,
> Vladimir
>
> On 3/24/14 3:10 AM, Schmidt, Lutz wrote:
>> Hi Vladimir,
>>
>> your requests are reflected in the updated webrev: http://cr.openjdk.java.net/~goetz/webrevs/8037802/webrev.01/
>>
>> In detail:
>>    - output.cpp now contains comments where to find CallStubImpl and HandlerImpl.
>>    - For x86, all code related to CallStubImpl and HandlerImpl is now located in x86.ad.
>>      The code previously existing in x86_32.ad and x86_64.ad was also moved.
>>      (I like this variant much better - everything in one place. My previous variant tried
>>       to minimize #ifdefs)
>>    - "trampoline comments" in sparc.ad and x86.ad are now the same as in ppc.ad.
>>
>> Please let me know what you think.
>>
>> Thanks, Lutz
>>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Freitag, 21. März 2014 17:35
>> To: Schmidt, Lutz; hotspot-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
>> Subject: Re: RFR(M): 8037821: Account for trampoline stubs when estimating code buffer sizes
>>
>> Hi Lutz,
>>
>> I agree with these changes but I have few small comments.
>>
>> Add comment into output.cpp that CallStubImpl and HandlerImpl are defined in .ad files so we know where to look.
>> Since you are touching the x86 code, could you move HandlerImpl to x86.ad and use #ifdef _LP64 (and other LP64 macros)
>> for not matching code. I am not sure why we use call to push pc in x86_64.ad and not InternalAddress as in x86_32.ad but
>> leave it as it is.
>>
>> About trampoline comments in sparc.ad and x86.ad.
>> Remove "// call trampolines" lines and use the same comments as in ppc.ad because you have the explanation comment on
>> return statements:
>>
>> +  // Size of call trampoline stub.
>> +  static uint size_call_trampoline() {
>> +    return 0; // no call trampolines on this platform
>> +  }
>> +
>> +  // Number of relocations needed by a call trampoline stub.
>> +  static uint reloc_call_trampoline() {
>> +    return 0; // no call trampolines on this platform
>> +  }
>>
>> Note, I would need to change our closed sources accordingly after we agree on final version of changes.
>>
>> Thanks,
>> Vladimir
>>
>> On 3/20/14 3:51 AM, Schmidt, Lutz wrote:
>>> Hi all,
>>>
>>> I would welcome reviews for my change. I would need a sponsor as well.
>>>
>>> The initial reason for the change was fixing size calculations for stubs section and relocations (relevant for PPC port only). On PowerPC, far calls require "trampoline code" in the stubs section. The space needed for these trampoline stubs was not accounted for in shorten_branches processing. The additional relocation entries were "forgotten" as well. Correct a-priori calculation of the section sizes is desirable because it avoids the need for later resizing.
>>>
>>> To avoid platform-specific code in shorten_branches, I introduced a small class CallStubImpl in the architecture description (.ad) files. The class declaration makes its way to the ad_<arch>.hpp file which is included in share/vm/opto/output.cpp anyway. Because the size functions are contained in the class declaration, they are inlined at call sites (and optimized away in most cases).
>>>
>>> I took the opportunity to get rid of the extern declarations for
>>>      size_exception_handler()
>>>      size_deopt_handler()
>>> in share/vm/opto/output.cpp altogether. Using the trampoline stubs approach as a template, I introduced another small class HandlerImpl in the architecture description (.ad) files. It declares and defines the size and emitter functions for the exception and depot handlers. Program logic is not changed.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~goetz/webrevs/8037802/webrev.00/
>>>
>>> Testing:
>>> Automated tests, including SPEC benchmarks and the JCK suite, did not reveal any issues.
>>>
>>> Thanks a lot!
>>> Lutz
>>>
>>> Lutz Schmidt | SAP AG | Walldorf, Germany
>>>