On Thu, 23 May 2024 16:33:14 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
>> As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we >> are not consistently setting all assembler directives correctly, we can get >> errors that are not detected by the linker. >> >> We should stop duplicating this code and create a unified macro to properly >> setup functions, and use it everywhere. > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains five commits: > > - Merge branch 'master' into hotspot-assembler-functions > - Fix copyright headers > - Fix building on macos/aarch64 > - Use % instead of @ due to arm assembler > - 8331921: Hotspot assembler files should use common logic to setup exported > functions What testing has been done in our CI? The changes seem quite reasonable but it would be easy to overlook a small difference, so the proof of this is in the building and testing. One query below, but hitting approve. Thanks make/hotspot/lib/CompileJvm.gmk line 53: > 51: > -I$(TOPDIR)/src/hotspot/os_cpu/$(HOTSPOT_TARGET_OS)_$(HOTSPOT_TARGET_CPU_ARCH) > \ > 52: -I$(TOPDIR)/src/hotspot/os/$(HOTSPOT_TARGET_OS) \ > 53: -I$(TOPDIR)/src/hotspot/os/$(HOTSPOT_TARGET_OS_TYPE) \ What does the second line evaluate to on Windows? Does it duplicate the prior line? ------------- Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19138#pullrequestreview-2081640019 PR Review Comment: https://git.openjdk.org/jdk/pull/19138#discussion_r1616510889
