Arch specific changes "platform-specific"

Dmitry Samersoff dms at samersoff.net
Fri Sep 13 08:32:56 UTC 2019


Hello David,

> So normally watching for changes under "src/hotspot/cpu" should be
> enough. I'd like to keep things easy (read "lazy"), and in general
> it is easy to forget special labelling (we are currently using
> "lworld" and "lw<x>" just to avoid using a separate project).

It's exactly what I'm doing now and, unfortunately, it not always helps.

I would like to track changes on *CR level*, not on source code one.

i.e.  set-up JIRA dashboard, check daily what is happening and then add
myself to watchers of relevant issues and select it from review flow to
catch up with upcoming changes.

I'm Ok with human errors - ever partial simplification will have a value.

-Dmitry



On 12.09.19 12:18, David Simms wrote:
> 
> To aid with current and future porting work...
> 
> So normally watching for changes under "src/hotspot/cpu" should be
> enough. I'd like to keep things easy (read "lazy"), and in general it is
> easy to forget special labelling (we are currently using "lworld" and
> "lw<x>" just to avoid using a separate project).
> 
> But there are occasions when changes are arch-specific and don't touch
> any of these files. In which case, yes we could be helpful and add a
> label, to point out potential platform issues or attention required.
> 
>     E.g. field layout changes might be a good example. Changes happen in
> platform independent code, but often have consequences for different
> platforms.
> 
> Proposal:
> 
>     Anyone wanting to flag potential platform specific issues simply add
> "platform-specific" (existing label). Best effort, doesn't preclude
> folks making obvious platform specific changes and forgetting the label.
> 
> Feel free to come with better ideas (keeping in mind, the least amount
> for "process" is always goal).
> 
> Cheers
> /David Simms
> 
> 
> On 12/09/19 9:56 AM, Dmitry Samersoff wrote:
>> Frederic,
>>
>>> I’ve made the CR x86 specific, and also added [x86] to the summary.
>>> Do you need a more specific label?
>> What I need is a clear indication that the fix contains platform
>> specific code and therefor have to be ported (or validated) to other
>> platforms like AArch64.
>>
>> Label seems to me the most natural way to provide such indication, but
>> any other approach will work for me as soon as I can easy select all
>> changes for period that requires my attention.
>>
>> -Dmitry
>>
>> On 11.09.19 17:13, Frederic Parain wrote:
>>> Dmitry,
>>>
>>> Thank you for the review.
>>>
>>> I’ve moved the verify_oop() as suggested:
>>>
>>> http://cr.openjdk.java.net/~fparain/opt_int/webrev.01/index.html
>>>
>>> I’ve made the CR x86 specific, and also added [x86] to the summary.
>>> Do you need a more specific label?
>>>
>>> Regards,
>>>
>>> Fred
>>>
>>>
>>>> On Sep 11, 2019, at 09:16, Dmitry Samersoff <dms at samersoff.net> wrote:
>>>>
>>>> Hello Frederic,
>>>>
>>>> Looks good to me.
>>>>
>>>> You may consider to move
>>>>
>>>> __ verify_oop(rax);
>>>>
>>>> after done, to save few instructions.
>>>>
>>>> PS:
>>>>
>>>> Could you add a label (something like x86_specific) to the CR to
>>>> simplify further porting.
>>>>
>>>> We discussed this approach with IoI and it may be a time to start
>>>> adopting it.
>>>>
>>>> -Dmitry
>>>>
>>>> On 11.09.19 15:20, Frederic Parain wrote:
>>>>> Please this review this small change in the interpreter adding
>>>>> a fast path to defaultvalue.
>>>>>
>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8230851
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~fparain/opt_int/webrev.00/index.html
>>>>>
>>>>> Thank you,
>>>>>
>>>>> Fred
>>>>>
> 


More information about the valhalla-dev mailing list