Request for Review (XS): 7087445, Improve platform independence of JSR292 shared code

[David Holmes]

If set_initial_fp is not actually setting a fp at all then it should be renamed.

My 2c.

David

On 7/09/2011 5:39 PM, Bertrand Delsart wrote:
> Well, this is not an 'fp' an all platforms. In fact, what most platform
> might need is the unextended_sp of the initial caller frame (which just
> happens to be stored in FP on x86's compiled frames).
>
> Thus, if the conflict between the field name and the method bother you, I
> would prefer s/initial_fp/initial_info/ for the field and its accessors.
>
> I didn't do it before to minimize changes (e.g. not touch the
> deoptimization code that reads the field in sharedRuntime_x86_[32|64].cpp))
> but the idea is really that deoptimization.cpp need not know what this data
> is (and should not assume it does).
>
> Another solution, ti make the platform dependency explicit, is:
> - s/initial_fp/pd_data/ (for platform dependent field name and accessors).
> - s/initial_deoptimization_info/deoptimization_pd_data/ (for the platform
> dependent call filling the field)
>
> Do you prefer me to make this changes which will also impact the
> sharedRuntime_... files ? Any preference on the naming (info|pd_data) ?
>
> Bertrand.
>
> On 09/ 7/11 01:38 AM, John Rose wrote:
>> It's a little jarring to have initial_deoptimization_info supply the
>> initial_fp of the unroll block.
>>
>> I suggest s/initial_deoptimization_info/initial_fp_for_deoptimization/ to
>> simplify the correspondence.
>>
>> Then platforms which supply fp() for this don't have to apologize so much,
>> and this line looks more normal:
>>
>> info->set_initial_fp((intptr_t)
>> array->sender().initial_fp_for_deoptimization());
>>
>>
>> -- John
>>
>> On Sep 6, 2011, at 9:04 AM, Bertrand Delsart wrote:
>>
>>> Small shared changes necessary to improve portability of jsr292
>>> on some platforms.
>>>
>>> http://cr.openjdk.java.net/~bdelsart/7087445/webrev.00/
>>>
>>> Should have no impact on the existing ports, as long as
>>> you add this backward compatible definition (added to SPARC,
>>> x86 and zero):
>>>
>>> intptr_t *frame::initial_deoptimization_info() {
>>> return fp();
>>> }
>>>
>>> Thanks,
>>>
>>> Bertrand.
>>> --
>>> Bertrand Delsart, bertrand.delsart at oracle.com,
>>> Sun-Oracle France, 180 av. de l'Europe, ZIRST de Montbonnot,
>>> 38334 Saint Ismier, FRANCE
>>
>
>