[aarch64-port-dev ] [8u] RFR (M) 8228400: Remove built-in AArch64 simulator

Aleksey Shipilev shade at redhat.com
Mon Aug 5 10:38:54 UTC 2019


On 8/5/19 11:42 AM, Andrew Dinn wrote:
> There are also a few 'equivalent' removals but to code that has is now
> in a different source file:
> 
>   relocate of templateInterpreter code to templateInterpreterGenerator
>   relocate of interpreter code to templateInterpreterGenerator
> 
> The specific changes all look correct. So, I don't think there is any
> problem with the patch ... well, except you have omitted  updates to
> quite a few copyright headers :-]

Right. There no strict rule here: backporting work does not usually make additional copyright dates
changes, unless the backported changeset demands it, or there is a conflict. Removals are even more
awkward: consider what happens if the removed code had come in the single changeset. Then it is the
usual practice to cleanly backout the complete changeset, even years later, keeping copyright dates
untouched.

In the 8u-aarch64 case, I think we would need to bulk update copyright headers before 8u upstreaming
anyway, so it does not hurt either way. We have also did not do updates in either jdk/jdk changeset
(already pushed), jdk11u-dev webrev (can still be amended), or here. So, I'd rather keep the
removals without additional changes. Can update the copyrights if you insist.

> However, this left me wondering why these new changes had turned up. It
> seems some of these new removals make sense but a couple appear to have
> been missed from the corresponding 11u files:
> 
>  3) os_linux_aarch64.cpp

Not following:

8u:
https://cr.openjdk.java.net/~shade/8228400/webrev.8u.01/src/os_cpu/linux_aarch64/vm/os_linux_aarch64.cpp.sdiff.html
11u:
https://cr.openjdk.java.net/~shade/8228400/webrev.11u.01/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp.sdiff.html

The diff is nearly the same. Changed parts are in SPELL_REG_{SP,FP}, and one BUILTIN_SIM block is
not removed, because there is no such block in 11u.

>  4) c1_Runtime1_aarch64.cpp

Not following:

8u:
https://cr.openjdk.java.net/~shade/8228400/webrev.8u.01/src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp.sdiff.html
11u:
https://cr.openjdk.java.net/~shade/8228400/webrev.11u.01/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp.sdiff.html

The diff is the same.

> So, sorry, a jdk11u follow-up is needed. Also, I think quite a few of
> the copyrights did not get updated in the jdk11u patch :-/

See above re: copyright dates. Otherwise, omissions from 11u patch should really be discussed in 11u
review thread, which is still open.

-- 
Thanks,
-Aleksey



More information about the aarch64-port-dev mailing list