Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement os/cpu for macOS/AArch64 (2) #24

Open
wants to merge 8 commits into
base: macos-aarch64
Choose a base branch
from

Conversation

KaperD
Copy link

@KaperD KaperD commented Dec 8, 2021

Это я сделал cherry-pick патча без лишних коммитов, и потом сделал небольшие исправления. С предыдущим патчем отличия в копирайтах и в паре мест в стиле. + небольшое отличие os_bsd_aarch64.cpp. + пока не взял изменения из openjdk/jdk@a9452a4

Коммиты из патча, которые я считаю ненужными для нас:

  1. JDK-8262491: bsd_aarch64 part
    JDK-8262491 не бекпортирован в jdk15, поэтому этого метода там нет

  2. JDK-8263002: bsd_aarch64 part
    JDK-8263002 не бекпортирован в jdk15, поэтому это изменение не нужно

  3. JDK-8259937: bsd_aarch64 part
    JDK-8259937 не бекпортирован в jdk15

  4. JDK-8260471: bsd_aarch64 part
    JDK-8260471 не бекпортирован в jdk15, поэтому это изменение не нужно

  5. Fix after JDK-8259539, partially revert preconditions
    JDK-8259539 не бекпортирован в jdk15. Однако такое изменение всё равно можно взять к нам, так как оно может решить проблему с warning-ами, которые сейчас приходится явно отключать прямо в cpp коде

  6. JDK-8259539: bsd_aarch64 part
    Bug-id такой же, как и у прошлого коммита. Тоже не нужен, так как этот патч нет бекпортировали в jdk15

  7. JDK-8257828: bsd_aarch64 part
    JDK-8257828 не бекпортировали в jdk15. Однако его бекпортировали в jdk11. Может быть и нам тогда стоит, но тогда нужно ещё перед этим рефакторинг обработки сигналов бекпортировать, что уже не легко скорее всего

  8. Removed unused variables
    Связан с 5) и 6), поэтому просто не нужен

  9. Pull/2200 (openjdk#5)
    Часть изменений точно не нужна (убрали workaround for Mavericks, но он нужен в jdk15, так как там минимальная версия MacOS как раз Mavericks). Но остальное можно и взять

  10. Update signal handler part for debugger
    Такого файла нет в jdk15. Надо понять нужно ли делать эти изменения в другом файле

  11. JDK-8253742: bsd_aarch64 part
    JDK-8253742 не бекпортирован в jdk15, поэтому изменения не нужны

  12. JDK-8257882: bsd_aarch64 part
    JDK-8257882 не бекпортирован в jdk15. Изменение связано с коммитом 3)

  13. JDK-8221554: bsd_aarch64 part
    JDK-8221554 не бекпортирован в jdk15. Однако, может быть стоит его бекпортировать

  14. Fix merge
    Как я понимаю, изменение связано с JDK-8255711, его не бекпортировали в jdk15

@KaperD
Copy link
Author

KaperD commented Dec 9, 2021

Сюда ещё сразу попала часть WX в os_bsd_aarch64.cpp. Может быть стоит её отсюда убрать

@KaperD
Copy link
Author

KaperD commented Dec 9, 2021

Про 10: вроде нужно в этом файле изменить

#if defined(__APPLE__)

@AntonKozlov
Copy link
Member

Это я сделал cherry-pick патча без лишних коммитов

Из https://github.com/KaperD/jdk/commits/f45876cc9ef74b675370904f3d7985a7ca440cae ?

А как бы мне сравнить то что в этом PR и состояние f45876cc9e? Т.е. я мог бы посмотреть

git diff dbc9e4b50 f45876cc9e -- <пути>

Но в каких путях?


  1. Fix after JDK-8259539, partially revert preconditions
    JDK-8259539 не бекпортирован в jdk15. Однако такое изменение всё равно можно взять к нам, так как оно может решить проблему с warning-ами, которые сейчас приходится явно отключать прямо в cpp коде

Согласен, я бы взял. Но JDK-8255711 мы вряд ли захотим и сможем

  1. JDK-8257828: bsd_aarch64 part
    JDK-8257828 не бекпортировали в jdk15. Однако его бекпортировали в jdk11. Может быть и нам тогда стоит, но тогда нужно ещё перед этим рефакторинг обработки сигналов бекпортировать, что уже не легко скорее всего

Действительно, будет тяжело. JDK-8257828 потенциально приятное, но не обязательно. От него легко отказаться в нашем бекпор
те

  1. Removed unused variables
    Связан с 5) и 6), поэтому просто не нужен

Т.е. точно не берём 5 ?

  1. Pull/2200 (openjdk#5)
    Часть изменений точно не нужна (убрали workaround for Mavericks, но он нужен в jdk15, так как там минимальная версия MacOS как раз Mavericks). Но остальное можно и взять

Согласен, c_calling_convention_priv надо брать

  1. Update signal handler part for debugger
    Такого файла нет в jdk15. Надо понять нужно ли делать эти изменения в другом файле

Точно надо, без этого не работает дебаггер должным образом. В 15u этот блок в os_bsd.cpp

EXC_MASK_BAD_ACCESS | EXC_MASK_ARITHMETIC,

  1. JDK-8221554: bsd_aarch64 part
    JDK-8221554 не бекпортирован в jdk15. Однако, может быть стоит его бекпортировать

Мы наверно должны остаться согласованными с другими реализациями */aarch64, т.е. оставить как есть:

inline void OrderAccess::cross_modify_fence() { }

inline void OrderAccess::cross_modify_fence() { }

  1. Fix merge
    Как я понимаю, изменение связано с JDK-8255711, его не бекпортировали в jdk15

Частично, в os_bsd_aarch64.cpp.

В vm_version_bsd_aarch64.cpp изменение может быть полезным.

@KaperD
Copy link
Author

KaperD commented Dec 9, 2021

KaperD/jdk@5e4dae9 вот этот коммит брал. В этом патче все файлы в src/hotspot/os_cpu/bsd_aarch64/ ну и ещё 3 мелких изменения в других файлах

@KaperD
Copy link
Author

KaperD commented Dec 9, 2021

Т.е. точно не берём 5 ?

5 можно самим сделать, потому что именно этим коммитом не получится, так как он чисто не наложится

@KaperD
Copy link
Author

KaperD commented Dec 9, 2021

Мы наверно должны остаться согласованными с другими реализациями */aarch64, т.е. оставить как есть:

Я имел в виду, что можно сам патч бекпортировать, тогда в других реализациях это тоже изменится

@KaperD
Copy link
Author

KaperD commented Dec 9, 2021

В vm_version_bsd_aarch64.cpp изменение может быть полезным.

Там исправляется метод, который не нужен в jdk15 (SVE нет в jdk15), поэтому вроде не будет полезным

@AntonKozlov
Copy link
Member

Мы наверно должны остаться согласованными с другими реализациями */aarch64, т.е. оставить как есть:

Я имел в виду, что можно сам патч бекпортировать, тогда в других реализациях это тоже изменится

https://bugs.openjdk.java.net/browse/JDK-8221554 выглядит слишком большим. Давай не будем.

@AntonKozlov
Copy link
Member

Я что-то увлёкся и перебазировал твою ветку коммитов, взяв 9 и 10.
https://github.com/AntonKozlov/jdk/tree/pr-2200-rebase
соответственно состояние из которого можно было бы тащить -- AntonKozlov/jdk@6821e84.
Наверно ты мог бы сделать так же.

Но ещё было бы круто отбазировать этот PR поверх коммита c3fc678

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants