Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 9 Jun 2026 10:22:05 +0200
From:      Piotr Kubaj <pkubaj@FreeBSD.org>
To:        Kirill Ponomarev <kp@krion.cc>
Cc:        ports-committers@freebsd.org, dev-commits-ports-all@freebsd.org, dev-commits-ports-main@freebsd.org
Subject:   Re: git: b2d77d05989f - main - lang/sbcl: add powerpc64 support
Message-ID:  <aifNLdRbHBv3o8gs@talos-powerpc64le>
In-Reply-To: <aiVtoeDaXVIXTsek@krion.cc>
References:  <6a25368f.1d089.61a69c14@gitrepo.freebsd.org> <aiVtoeDaXVIXTsek@krion.cc>

index | next in thread | previous in thread | raw e-mail

[-- Attachment #1 --]
Since you also replied here, I'm posting my answer from the PR:
The original powerpc64le patch had a number of Makefile changes that should ideally be reviewed, in case you preferred them to be written another way. However, powerpc64 addition contains none of those. The Makefile part is simply adding powerpc64 to ONLY_FOR_ARCHS and adding a bootstrap entry. All the patches are related to the ppc64 SBCL backend and only change the code that doesn't even affect powerpc64le (only big-endian). Moreover, since they are upstreamed, they will be part of the next release anyway. So while powerpc64le patch contained some important decisions towards port's Makefile, powerpc64 patch has none of it. After the next upgrade, only the Makefile part will stay. I think it's fine to say it's under portmgr blanket for tier 2 architectures.

On 26-06-07 13:09:53, Kirill Ponomarev wrote:
> Hi Piotr,
> 
> Unlike your powerpc64le change in January, which went through the
> maintainer-approval flag and was committed after a proper timeout,
> this commit was pushed without asking for approval at all — the patch
> was never attached to the PR for review, there is no Approved-by line
> in the commit message, and I learned about the change only after the
> fact.
> 
> As you know, lang/sbcl is a maintained port, and adding support for a
> new architecture is clearly not covered by any blanket approval. Per
> the Committer's Guide, changes like this require explicit maintainer
> approval or a timeout after a review request.
> 
> I'm not against powerpc64 support as such, but I do want to review
> changes to the port before they land — especially non-trivial ones
> touching the bootstrap logic and adding several runtime patches.
> Please follow the normal approval process for lang/sbcl in the future:
> attach the patch to the PR, set the maintainer-approval flag, and give
> me a chance to look at it.
> 
> On 06/07, Piotr Kubaj wrote:
> > The branch main has been updated by pkubaj:
> > 
> > URL: https://cgit.FreeBSD.org/ports/commit/?id=b2d77d05989f2c8dba36f2aba6f39002a1c26a84
> > 
> > commit b2d77d05989f2c8dba36f2aba6f39002a1c26a84
> > Author:     Piotr Kubaj <pkubaj@FreeBSD.org>
> > AuthorDate: 2026-06-06 19:30:11 +0000
> > Commit:     Piotr Kubaj <pkubaj@FreeBSD.org>
> > CommitDate: 2026-06-07 09:13:25 +0000
> > 
> >     lang/sbcl: add powerpc64 support
> >     
> >     All the patches are upstreamed, will be part of the next release.
> >     
> >     PR:     276877
> > ---
> >  lang/sbcl/Makefile                                 |  4 +-
> >  lang/sbcl/distinfo                                 |  4 +-
> >  lang/sbcl/files/patch-make-config.sh               | 19 +++++++
> >  .../files/patch-src_compiler_ppc64_c-call.lisp     | 59 ++++++++++++++++++++++
> >  .../sbcl/files/patch-src_compiler_ppc64_parms.lisp | 13 +++++
> >  lang/sbcl/files/patch-src_runtime_ppc-arch.c       | 22 ++++++++
> >  lang/sbcl/files/patch-src_runtime_ppc64-assem.S    | 21 ++++++++
> >  7 files changed, 139 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lang/sbcl/Makefile b/lang/sbcl/Makefile
> > index 9ada17552809..12b01dc607d7 100644
> > --- a/lang/sbcl/Makefile
> > +++ b/lang/sbcl/Makefile
> > @@ -20,7 +20,7 @@ LICENSE=	BSD2CLAUSE PD
> >  LICENSE_COMB=	dual
> >  LICENSE_FILE=	${WRKSRC}/COPYING
> >  
> > -ONLY_FOR_ARCHS=	amd64 i386 powerpc64le
> > +ONLY_FOR_ARCHS=	amd64 i386 powerpc64 powerpc64le
> >  
> >  LIB_DEPENDS=	libgmp.so:math/gmp \
> >  		libmpfr.so:math/mpfr
> > @@ -111,7 +111,7 @@ MAKE_SH_ARGS?=	--prefix="${PREFIX}" --xc-host="${XC_HOST}"
> >  MAKE_SH_ARGS+=	--dynamic-space-size=${DYNAMIC_SPACE_SIZE}
> >  .endif
> >  SBCL_BOOT_LIST=	1.2.7-x86-64-freebsd 1.2.7-x86-freebsd 1.2.7-x86-64-dragonfly \
> > -		2.5.7-powerpc64le-freebsd
> > +		2.5.7-powerpc64le-freebsd 2.6.5-powerpc64-freebsd
> >  
> >  .include <bsd.port.options.mk>
> >  
> > diff --git a/lang/sbcl/distinfo b/lang/sbcl/distinfo
> > index 5783026ec20f..582c0daaf839 100644
> > --- a/lang/sbcl/distinfo
> > +++ b/lang/sbcl/distinfo
> > @@ -1,6 +1,8 @@
> > -TIMESTAMP = 1780160862
> > +TIMESTAMP = 1780741067
> >  SHA256 (sbcl-2.6.5-source.tar.bz2) = 91ec75f647252ed6e6aeae9b1a13f47c7c6cfd9b68488dc69f1a6fea5accb440
> >  SIZE (sbcl-2.6.5-source.tar.bz2) = 8497853
> > +SHA256 (sbcl-2.6.5-powerpc64-freebsd-binary.tar.bz2) = 524bd7a65f9b499dd32be87438118893c9f74976e33912920d1ab86b1a509425
> > +SIZE (sbcl-2.6.5-powerpc64-freebsd-binary.tar.bz2) = 8592925
> >  SHA256 (sbcl-1.2.7-x86-64-freebsd-binary.tar.bz2) = c61f5e777e56921d2452d0fa6b71024ccd9b99bc659676498d398b8663176492
> >  SIZE (sbcl-1.2.7-x86-64-freebsd-binary.tar.bz2) = 10463348
> >  SHA256 (sbcl-1.2.7-x86-freebsd-binary.tar.bz2) = cf68bfab780a14964d9593f5b47fa3e174cf43e95ae3e558712d218f1c37bdbe
> > diff --git a/lang/sbcl/files/patch-make-config.sh b/lang/sbcl/files/patch-make-config.sh
> > new file mode 100644
> > index 000000000000..2b821e305daa
> > --- /dev/null
> > +++ b/lang/sbcl/files/patch-make-config.sh
> > @@ -0,0 +1,19 @@
> > +--- make-config.sh.orig
> > ++++ make-config.sh
> > +@@ -765,6 +765,16 @@
> > +     fi
> > +     ;;
> > +   ppc64)
> > ++    # The ppc64 C calling convention is either ELFv1, which passes function
> > ++    # pointers as 3-word descriptors, or ELFv2, which branches to the entry
> > ++    # address directly.  This is a property of the toolchain, independent of
> > ++    # endianness (ppc64le is always ELFv2; ppc64 big-endian may be either), so
> > ++    # ask the C compiler which ABI it targets via its _CALL_ELF predefine
> > ++    # (2 = ELFv2, 1 or undefined = ELFv1) and add :ppc64-elfv1 for the
> > ++    # descriptor ABI.
> > ++    if [ "`echo | ${CC:-cc} -E -dM - 2>/dev/null | grep -w _CALL_ELF | awk '{print $3}'`" != 2 ]; then
> > ++        printf ' :ppc64-elfv1' >> $ltf
> > ++    fi
> > +    ;;
> > +   riscv)
> > +     if [ "$xlen" = "64" ]; then
> > diff --git a/lang/sbcl/files/patch-src_compiler_ppc64_c-call.lisp b/lang/sbcl/files/patch-src_compiler_ppc64_c-call.lisp
> > new file mode 100644
> > index 000000000000..57d2e85ec47c
> > --- /dev/null
> > +++ b/lang/sbcl/files/patch-src_compiler_ppc64_c-call.lisp
> > @@ -0,0 +1,59 @@
> > +--- src/compiler/ppc64/c-call.lisp.orig
> > ++++ src/compiler/ppc64/c-call.lisp
> > +@@ -25,7 +25,7 @@
> > + ;;;; "The stack pointer (stored in r1) shall maintain quadword alignment."
> > + ;;;; (quadword = 16 bytes)
> > + (defconstant +stack-alignment-mask+ 15)
> > +-(defconstant +stack-frame-size+ #+little-endian 12 #+big-endian 14)
> > ++(defconstant +stack-frame-size+ #-ppc64-elfv1 12 #+ppc64-elfv1 14)
> > + 
> > + (defstruct arg-state
> > +   (gpr-args 0)
> > +@@ -139,7 +139,7 @@
> > +         (arg-tns (invoke-alien-type-method :arg-tn arg-type arg-state)))
> > +       (values (make-wired-tn* 'positive-fixnum any-reg-sc-number nsp-offset)
> > +               (let ((size (arg-state-stack-frame-size arg-state)))
> > +-                (cond #+little-endian
> > ++                (cond #-ppc64-elfv1
> > +                       ((= size +stack-frame-size+)
> > +                        ;; no stack args
> > +                        0)
> > +@@ -261,7 +261,7 @@
> > +            (make-fpr (n)
> > +              (make-random-tn (sc-or-lose 'double-reg) n)))
> > +       (let* ((segment (make-segment))
> > +-             #+big-endian
> > ++             #+ppc64-elfv1
> > +              (function-descriptor-size 24))
> > +         (assemble (segment 'nil)
> > +           ;; Copy args from registers or stack to new position
> > +@@ -387,7 +387,7 @@
> > +                               (bug "Unknown alien floating point type: ~S" type))))))
> > +               ;; Leave a gap for a PPC64ELF ABIv1 function descriptor,
> > +               ;; to be filled in later relative to the SAP.
> > +-              #+big-endian
> > ++              #+ppc64-elfv1
> > +               (dotimes (k (/ function-descriptor-size 4)) ; nop is 4 bytes
> > +                 (inst nop))
> > +               (mapc #'save-arg
> > +@@ -411,9 +411,9 @@
> > +               (inst stdu stack-pointer stack-pointer (- frame-size))
> > + 
> > +               ;; And make the call.
> > +-              #+little-endian
> > ++              #-ppc64-elfv1
> > +               (load-address-into r0 (callback_wrapper_trampoline))
> > +-              #+big-endian
> > ++              #+ppc64-elfv1
> > +               (destructuring-bind (r2 r12) (mapcar #'make-gpr '(2 12))
> > +                 (load-address-into r12 (callback_wrapper_trampoline))
> > +                 (inst ld r0 r12 0)
> > +@@ -461,7 +461,7 @@
> > +           ;; instruction of the wrapper.  This assembler wrapper only
> > +           ;; cares about the address, so leave the other descriptor
> > +           ;; fields filled with no-op instructions.
> > +-          #+big-endian
> > ++          #+ppc64-elfv1
> > +           (setf (sap-ref-64 sap 0) (+ (sap-int sap) function-descriptor-size))
> > +           (alien-funcall
> > +            (extern-alien "ppc_flush_icache"
> > diff --git a/lang/sbcl/files/patch-src_compiler_ppc64_parms.lisp b/lang/sbcl/files/patch-src_compiler_ppc64_parms.lisp
> > new file mode 100644
> > index 000000000000..f3839b126d59
> > --- /dev/null
> > +++ b/lang/sbcl/files/patch-src_compiler_ppc64_parms.lisp
> > @@ -0,0 +1,13 @@
> > +--- src/compiler/ppc64/parms.lisp.orig
> > ++++ src/compiler/ppc64/parms.lisp
> > +@@ -80,7 +80,9 @@
> > +                               :dynamic-space-start #x1000000000)
> > + 
> > + (defconstant alien-linkage-table-growth-direction :up)
> > +-(defconstant alien-linkage-table-entry-size #+little-endian 28 #+big-endian 24)
> > ++;; ELFv2 uses a 7-instruction inline jump (28 bytes); the ELFv1 ABI uses
> > ++;; a 3-word function descriptor (24 bytes).
> > ++(defconstant alien-linkage-table-entry-size #-ppc64-elfv1 28 #+ppc64-elfv1 24)
> > + 
> > + 
> > + (defenum (:start 8)
> > diff --git a/lang/sbcl/files/patch-src_runtime_ppc-arch.c b/lang/sbcl/files/patch-src_runtime_ppc-arch.c
> > new file mode 100644
> > index 000000000000..039a0841fb11
> > --- /dev/null
> > +++ b/lang/sbcl/files/patch-src_runtime_ppc-arch.c
> > @@ -0,0 +1,22 @@
> > +--- src/runtime/ppc-arch.c.orig
> > ++++ src/runtime/ppc-arch.c
> > +@@ -616,7 +616,9 @@
> > +   // but trick the compiler into thinking it isn't, so that it does not
> > +   // indirect through a descriptor, but instead we get its logical address.
> > +   if (target_addr != &call_into_c) {
> > +-#ifdef LISP_FEATURE_LITTLE_ENDIAN
> > ++#ifndef LISP_FEATURE_PPC64_ELFV1
> > ++      /* ELFv2: no function descriptors, so the linkage entry is an inline
> > ++       * jump that materializes the target address in r12 and branches to it. */
> > +       int* inst_ptr;
> > +       unsigned long a0,a16,a32,a48;
> > +       unsigned int inst;
> > +@@ -670,7 +672,7 @@
> > +       os_flush_icache((os_vm_address_t) reloc_addr, (char*) inst_ptr - reloc_addr);
> > + 
> > + #else
> > +-      // Could use either ABI, but we're assuming v1
> > ++      /* ELFv1: function pointers are descriptors, as detailed below. */
> > +       /* In the 64-bit v1 ABI, function pointers are alway passed around
> > +        * as "function descriptors", not directly the jump target address.
> > +        * A descriptor is 3 words:
> > diff --git a/lang/sbcl/files/patch-src_runtime_ppc64-assem.S b/lang/sbcl/files/patch-src_runtime_ppc64-assem.S
> > new file mode 100644
> > index 000000000000..fd5798afa5ac
> > --- /dev/null
> > +++ b/lang/sbcl/files/patch-src_runtime_ppc64-assem.S
> > @@ -0,0 +1,21 @@
> > +--- src/runtime/ppc64-assem.S.orig
> > ++++ src/runtime/ppc64-assem.S
> > +@@ -259,7 +259,7 @@
> > + 
> > + 	/* "When a function is entered through its global entry point,
> > + 	 * register r12 contains the entry-point address." */
> > +-#ifdef LISP_FEATURE_BIG_ENDIAN
> > ++#ifdef LISP_FEATURE_PPC64_ELFV1
> > + 	mfctr 11
> > + 	ld reg_CFUNC, 0(11)
> > + 	/* In the v1 64-bit ABI, a function pointer is a pointer to a
> > +@@ -273,6 +273,9 @@
> > + 	// ld 11, 16(11)
> > + 	mtctr reg_CFUNC
> > + #else
> > ++	/* ELFv2: the call target is the function's address itself; reg_CFUNC
> > ++	 * is r12, which the global entry point uses to set up its own TOC.
> > ++	 * No descriptor indirection. */
> > + 	mfctr reg_CFUNC
> > + #endif
> > +         /* Into C we go. */
> > 



[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----

iQJPBAABCAA5FiEEycyIeNkkgohzsoorelmbhSCDnJ0FAmonzS0bFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMCwyAAoJEHpZm4Ugg5yd02IQAL7cSNH3glWeSs13iBZc
J0m1oZhi9Rv6MmjWy/LL0U0DVgesy5+7XkpswDLFsGTmWEpTfZyS5dRajeQ79XCh
OP+s0Gm36wOKgBYDFgAT7PGpS+knTip/ujVb6YvZUbvgOXjsCA+rWwuagFpdG0XZ
tBoijuB43gEBVRS2iNIG4D0T/XFy7DBPDDWMyQRoPThsxglrsV9VdT7bo0dgD3n0
AMKNXpd+ra+gERnyNzAtF7SYmqGUvVALwPx6trn6DOMnL6XOj2Gz3oNC81Cjx7ns
DI0MSLO+khFLFVEJZ+s3yV/qnep05smXgzrW8Q+MewfMkJic2zOISeDPOpOD6NqO
WL+vli7s1jhTIXfCnZRSl1v3aUlugSkphkG0w9e2DqYbOLhYy6xz2IqYobtN6IfQ
EUZoUAV4s7WP3FxtBp4CWHeMdZEvuHVb5QeSfZ57QiYOFGZKPSl/TsbpkszcBJ0L
1baySlcgOReJyRHk0hZfQ0M0Ht5t4hBFzY7f85yKp19f/Jh221AFm4A93PG5ZZ4v
2TNp5TAeHpYw/eQx9c5M0A6dAqdx7CMPltDKpgz0u2fwGbw10ekSRpFfHqAOPxZy
7cdF+mvUBriblra/iq7EOTHzJwKNY+BuF7yYjlKzw8U/HE2FfTGjBSFP4lGIeKUd
IwLrP7MreXuTqeQ9YW7WfOU6
=/Brr
-----END PGP SIGNATURE-----
home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?aifNLdRbHBv3o8gs>