PowerPC {32, 64}-bit Block Trampolines #272

hmelder · 2024-01-20T17:18:30Z

Status

Besides UnexpectedException failing (just like on AArch64), all unit tests are passing. However, I would like to incorporate a proper fix for the hard-coded page sizes (See #271) as marking the whole page RWX is not really great... (edit: This was due to mprotect failing to set PROT_EXEC on an unaligned page as ppc64el has a page size of 64k on Debian)
This is why this PR is still marked as a draft.

It would also be good to have a CI workflow for PowerPC.

PowerPC 32-bit

System (QEMU):

OS: Debian GNU/Linux trixie/sid ppc
Host: PowerMac3,1
CPU: 7400 (1) @ 900MHz

Clang:

root@debian:~# clang --version
Debian clang version 16.0.6 (19)
Target: powerpc-unknown-linux-gnu

I needed to link objc to libatomic. Ideas on how to check this in CMake are welcome!

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4e080d8..d2a3d3a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -245,6 +245,7 @@ if (WIN32 AND NOT MINGW)
 endif()

 target_link_libraries(objc tsl::robin_map)
+target_link_libraries(objc atomic)

 set_target_properties(objc PROPERTIES
        LINKER_LANGUAGE C

Results:

100% tests passed, 0 tests failed out of 190

Total Test time (real) = 213.12 sec

The following tests did not run:
	115 - objc_msgSend (Skipped)
	116 - objc_msgSend_optimised (Skipped)
	117 - objc_msgSend_legacy (Skipped)
	118 - objc_msgSend_legacy_optimised (Skipped)
	151 - UnexpectedException (Skipped)
	152 - UnexpectedException_optimised (Skipped)
	153 - UnexpectedException_legacy (Skipped)
	154 - UnexpectedException_legacy_optimised (Skipped)
	171 - FastPathAlloc (Skipped)
	172 - FastPathAlloc_optimised (Skipped)

POWER9 (PowerPC 64-bit Little-Endian)

System (QEMU):

OS: Debian GNU/Linux trixie/sid ppc64le
Host: IBM pSeries (emulated by qemu)
CPU: POWER9 (architected) (4) @ 1.000GHz

Clang:

root@debian:~/libobjc2# clang --version
Debian clang version 16.0.6 (19)
Target: powerpc64le-unknown-linux-gnu

Results:

100% tests passed, 0 tests failed out of 190

Total Test time (real) = 116.38 sec

The following tests did not run:
	115 - objc_msgSend (Skipped)
	116 - objc_msgSend_optimised (Skipped)
	117 - objc_msgSend_legacy (Skipped)
	118 - objc_msgSend_legacy_optimised (Skipped)
	151 - UnexpectedException (Skipped)
	152 - UnexpectedException_optimised (Skipped)
	153 - UnexpectedException_legacy (Skipped)
	154 - UnexpectedException_legacy_optimised (Skipped)
	171 - FastPathAlloc (Skipped)
	172 - FastPathAlloc_optimised (Skipped)

block_to_imp.c

block_trampolines.S

hmelder · 2024-01-21T20:07:41Z

I cannot reproduce the build error from the powerpc-linux-gnu cross builder. It seems like the GitHub Ubuntu image has some additional packages in /usr/lib32 which are incorrectly used instead.

The powerpc64le-linux-gnu cross builder works just fine.

hmelder · 2024-01-21T22:50:42Z

@rmottola do you want to try the changes out on real hardware, or perhaps have a look at the ASM?

davidchisnall · 2024-01-22T10:18:10Z

Take a look at what we do in snmalloc for the hacks required to make PowerPC run correctly. The Debian / Ubuntu binfmt packages set it up with the wrong page size. It's very annoying.

asmconstants.h

hmelder · 2024-01-22T10:40:24Z

Take a look at what we do in snmalloc for the hacks required to make PowerPC run correctly.

Alright. Thank you :)

The Debian / Ubuntu binfmt packages set it up with the wrong page size. It's very annoying.

The weird thing is that it works on a fresh 22.04 aarch64 rootfs with powerpc cross.

Co-authored-by: David Chisnall <davidchisnall@users.noreply.github.com>

hmelder · 2024-01-22T15:27:06Z

Take a look at what we do in snmalloc for the hacks required to make PowerPC run correctly.

It seems that snalloc only has a powerpc64le configuration in the workflow, but the problem lies with PowerPC (32-bit, Big-Endian). We can skip the integration for now, as it is a legacy architecture.

rmottola · 2024-01-24T10:13:22Z

@rmottola do you want to try the changes out on real hardware, or perhaps have a look at the ASM?

Yes, I can test it on real hardware. Setting up thinks on my iBook G4 as a first test. Will report.

I can use:

Debian clang version 16.0.6 (19)
Target: powerpc-unknown-linux-gnu
Thread model: posix

processor       : 0
cpu             : 7447A, altivec supported

rmottola · 2024-01-24T10:15:36Z

Built completes of the branch ppc-block-trampoline.

I get a couple of warnings
/home/multix/code/libobjc2/sarray2.h:55:8: warning: variable length array folded to constant array as an extension [-Wgnu-folding-constant]

/home/multix/code/libobjc2/objc_msgSend.S:16:2: warning: objc_msgSend() not implemented for your architecture [-W#warnings] #warning objc_msgSend() not implemented for your architecture

rmottola · 2024-01-24T10:16:38Z

How should I run tests?

multix@iBookG4-14:~/code/libobjc2/Build$ make test
make: *** No rule to make target 'test'.  Stop

Co-authored-by: David Chisnall <davidchisnall@users.noreply.github.com>

hmelder · 2024-01-28T20:06:29Z

objc_method_cache_version is defined as an _Atomic(uint64_t). Can we make this architecture dependent?

davidchisnall · 2024-01-28T20:38:23Z

Hmm, that’s needed for the safe caching, but it isn’t useful on platforms where 64-bit atomic reads are expensive. It needs to be 64-bit to avoid overflows. We can probably just not enable it on PowerPC32.

hmelder · 2024-01-28T20:44:06Z

Sorry my question was malformed.

It needs to be 64-bit to avoid overflows.

This answers it.

We can probably just not enable it on PowerPC32.

I am not familiar with the performance differences of non-native atomic reads, but is it that big of a deal?
What would you use instead?

davidchisnall · 2024-02-03T09:04:46Z

I am not familiar with the performance differences of non-native atomic reads, but is it that big of a deal?

Non-native ones require acquiring a lock and then doing the read. This basically serialises them all and so will offset any potential benefit from caching.

What would you use instead?

Don't do lookup caching on PowerPC32. The compiler doesn't (yet?) do this automatically anyway.

hmelder · 2024-02-04T12:51:10Z

Don't do lookup caching on PowerPC32. The compiler doesn't (yet?) do this automatically anyway.

If I understand the lookup caching implementation correctly, the caller can opt in and cache a pointer to the current slot, checking whether the slot was invalidated before using it.

When you call objc_slot_lookup_version, the final
parameter is used to return either the current value of
objc_method_cache_version or 0 if the slot is uncacheable.

What about wrapping the atomic objc_method_cache_version operations in ifdefs (not including them on ppc32), and always write 0 to *version in objc_method_cache_version?

davidchisnall · 2024-02-04T12:54:13Z

What about wrapping the atomic objc_method_cache_version operations in ifdefs (not including them on ppc32), and always write 0 to *version in objc_method_cache_version?

Yup, I think the thing to do on PowerPC32 is to always write 0 to *version and to hide the declaration of objc_method_cache_version.

hmelder · 2024-02-04T12:55:30Z

And to hide the declaration of objc_method_cache_version.

But does the declaration of an _Atomic type pull in the libatomic library?

hmelder · 2024-02-04T12:55:54Z

I do not like conditionally hiding parts of the public API.

davidchisnall · 2024-02-04T15:23:41Z

Updates to it will pull in atomic things, we should simply remove it if we’re on a platform without 64-bit atomics.

…jc2 into ppc-block-trampoline

hmelder · 2024-02-12T09:31:12Z

Caching is now disabled on ppc32. All tests are passing.

100% tests passed, 0 tests failed out of 190

Total Test time (real) = 201.64 sec

The following tests did not run:
	115 - objc_msgSend (Skipped)
	116 - objc_msgSend_optimised (Skipped)
	117 - objc_msgSend_legacy (Skipped)
	118 - objc_msgSend_legacy_optimised (Skipped)
	151 - UnexpectedException (Skipped)
	152 - UnexpectedException_optimised (Skipped)
	153 - UnexpectedException_legacy (Skipped)
	154 - UnexpectedException_legacy_optimised (Skipped)
	171 - FastPathAlloc (Skipped)
	172 - FastPathAlloc_optimised (Skipped)
root@debian:~/libobjc2/build-ppc# uname -a
Linux debian 6.6.8-powerpc-smp #1 SMP Debian 6.6.8-1 (2023-12-22) ppc GNU/Linux

davidchisnall · 2024-02-12T09:52:48Z

LGTM, please do some squashing before you merge!

* Implement PowerPC block trampoline * Adjust pagesize on ppc64 * Skip UnexpectedException test for PowerPC * Move PAGE_SIZE to asmconstants.h * Use PAGE_SIZE and PAGE_SHIFT macros for PowerPC * Add ppc64el and powerpc qemu-crossbuild targets * Add NO_SAFE_CACHING definition and guards * Do not export objc_method_cache_version on ppc32 --------- Co-authored-by: David Chisnall <davidchisnall@users.noreply.github.com>

hmelder added 8 commits January 20, 2024 16:07

Implement PowerPC block trampoline

3664b7d

Adjust indentation

ea9b9f4

PowerPC64 Linux has a different pagesize by default

22e737c

Temporary page size workaround

8379d09

Fix lwz offset in PowerPC block trampoline

c4d6a5d

Adjust LOAD instruction PowerPC 32-bit to lwz

2db13a5

Adjust pagesize on ppc64

444c54e

Skip UnexpectedException test for PowerPC

0958563

davidchisnall reviewed Jan 21, 2024

View reviewed changes

block_to_imp.c Outdated Show resolved Hide resolved

block_trampolines.S Outdated Show resolved Hide resolved

hmelder added 9 commits January 21, 2024 19:26

Move PAGE_SIZE to asmconstants.h

ac06f53

Use PAGE_SIZE and PAGE_SHIFT macros for PowerPC

c3cc4e9

Add trailing newline to asmconstants.h

e088997

Add ppc64el and powerpc qemu-crossbuild targets

bf7ed5b

Use correct linker

6ab3c5c

Restrict LD_LIBRARY_PATH and link with atomic

9c4872f

Remove powerpc arch

53b418e

Link with libatomic on PowerPC

39a3776

Add powerpc and powerpc64 to detect_arch.c

1f925a5

hmelder marked this pull request as ready for review January 21, 2024 20:08

hmelder requested a review from davidchisnall January 21, 2024 21:14

davidchisnall reviewed Jan 22, 2024

View reviewed changes

asmconstants.h Outdated Show resolved Hide resolved

Update asmconstants.h

c26e311

Co-authored-by: David Chisnall <davidchisnall@users.noreply.github.com>

hmelder and others added 8 commits January 28, 2024 18:31

Add trailing newline to asmconstants.h

b432464

Add ppc64el and powerpc qemu-crossbuild targets

c47bf6a

Use correct linker

1496103

Restrict LD_LIBRARY_PATH and link with atomic

68334c9

Remove powerpc arch

bb46b6c

Link with libatomic on PowerPC

4124c3b

Add powerpc and powerpc64 to detect_arch.c

da39edb

Update asmconstants.h

ef781fd

Co-authored-by: David Chisnall <davidchisnall@users.noreply.github.com>

hmelder force-pushed the ppc-block-trampoline branch from c26e311 to ef781fd Compare January 28, 2024 17:31

hmelder added 4 commits February 12, 2024 08:41

Merge branch 'ppc-block-trampoline' of ssh://github.com/gnustep/libob…

62b6676

…jc2 into ppc-block-trampoline

Add NO_SAFE_CACHING definition and guards

0815f3d

Do not export objc_method_cache_version on ppc32

0750a99

Check if version ptr is valid before writing to it

c7eb3c0

davidchisnall approved these changes Feb 12, 2024

View reviewed changes

Merge branch 'master' into ppc-block-trampoline

62fe4b7

hmelder merged commit e882423 into master Feb 12, 2024
59 checks passed

hmelder deleted the ppc-block-trampoline branch February 12, 2024 11:09

qmfrederik mentioned this pull request Feb 27, 2024

Cut a 2.2.1 release? #281

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PowerPC {32, 64}-bit Block Trampolines #272

PowerPC {32, 64}-bit Block Trampolines #272

hmelder commented Jan 20, 2024 •

edited

Loading

hmelder commented Jan 21, 2024

hmelder commented Jan 21, 2024

davidchisnall commented Jan 22, 2024

hmelder commented Jan 22, 2024

hmelder commented Jan 22, 2024 •

edited

Loading

rmottola commented Jan 24, 2024 •

edited

Loading

rmottola commented Jan 24, 2024

rmottola commented Jan 24, 2024

hmelder commented Jan 28, 2024

davidchisnall commented Jan 28, 2024

hmelder commented Jan 28, 2024

davidchisnall commented Feb 3, 2024

hmelder commented Feb 4, 2024 •

edited

Loading

davidchisnall commented Feb 4, 2024

hmelder commented Feb 4, 2024

hmelder commented Feb 4, 2024

davidchisnall commented Feb 4, 2024

hmelder commented Feb 12, 2024

davidchisnall commented Feb 12, 2024

PowerPC {32, 64}-bit Block Trampolines #272

PowerPC {32, 64}-bit Block Trampolines #272

Conversation

hmelder commented Jan 20, 2024 • edited Loading

Status

PowerPC 32-bit

POWER9 (PowerPC 64-bit Little-Endian)

hmelder commented Jan 21, 2024

hmelder commented Jan 21, 2024

davidchisnall commented Jan 22, 2024

hmelder commented Jan 22, 2024

hmelder commented Jan 22, 2024 • edited Loading

rmottola commented Jan 24, 2024 • edited Loading

rmottola commented Jan 24, 2024

rmottola commented Jan 24, 2024

hmelder commented Jan 28, 2024

davidchisnall commented Jan 28, 2024

hmelder commented Jan 28, 2024

davidchisnall commented Feb 3, 2024

hmelder commented Feb 4, 2024 • edited Loading

davidchisnall commented Feb 4, 2024

hmelder commented Feb 4, 2024

hmelder commented Feb 4, 2024

davidchisnall commented Feb 4, 2024

hmelder commented Feb 12, 2024

davidchisnall commented Feb 12, 2024

hmelder commented Jan 20, 2024 •

edited

Loading

hmelder commented Jan 22, 2024 •

edited

Loading

rmottola commented Jan 24, 2024 •

edited

Loading

hmelder commented Feb 4, 2024 •

edited

Loading