[bug#74253] [PATCH] transformations: Add multituned-package.

[bug#74253] [PATCH] transformations: Add multituned-package.

[Efraim Flashner]

* guix/transformations.scm (package-tuned-for-psabi,
multituned-package): New variables.

Change-Id: I09ac7ae9fc2bcd9aa712b3c30fef807bc7d55895
---

This allows wrapping a package definition in multituned-package, ie:

(define-public opus
 (multituned-package
  (package
   ...)))

I'm not sure where to go with this patch from here. This will provide
the psabi libraries for x86_64 and powerpc64le so they get most of the
benefits from tuning for the architecture but without needing to specify
which architecture to tune for.  It should also provide a nice boost for
guix packs and docker images and the like.

The downside with using this by default is the larger package size due
to the extra versions of the libraries, and if it is used then the
regular --tune is disabled for that package.

I think adding it as a '--tune=generic' or '--tune=psabi' would be a
nice way to use it.

 guix/transformations.scm | 75 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/guix/transformations.scm b/guix/transformations.scm
index ea8b7a08443..4787e016b5d 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -60,6 +60,7 @@ (define-module (guix transformations)
 
             tunable-package?
             tuned-package
+            multituned-package
 
             show-transformation-options-help
             transformation-option-key?
@@ -634,6 +635,80 @@ (define (tuned-package p micro-architecture)
        ;; call 'tuned-package' again on this one.
        ,@(alist-delete 'tunable? (package-properties p))))))
 
+(define (package-tuned-for-psabi p psabi)
+  (let ((base (tuned-package p psabi)))
+    (package/inherit base
+      (name (string-append (package-name base) "-" psabi))
+      (arguments
+       (substitute-keyword-arguments (package-arguments base)
+         ((#:configure-flags flags #~'())
+          #~(append
+              (list
+                #$@(if (eq? (build-system-name (package-build-system p)) ; not base
+                            (quote cmake-build-system))
+                       #~((string-append "-DCMAKE_INSTALL_LIBDIR=lib/glibc-hwcaps/"
+                                         #$psabi))
+                       #~((string-append "--libdir=" #$output
+                                         "/lib/glibc-hwcaps/" #$psabi))))
+              #$flags))
+         ((#:phases phases #~%standard-phases)
+          #~(modify-phases #$phases
+              (add-after 'install 'remove-extra-files
+                (lambda _
+                  (for-each (lambda (dir)
+                              (when (file-exists? (string-append #$output dir))
+                                (delete-file-recursively
+                                  (string-append #$output dir))))
+                            (list (string-append "/lib/glibc-hwcaps/"
+                                                 #$psabi "/cmake")
+                                  (string-append "/lib/glibc-hwcaps/"
+                                                 #$psabi "/pkgconfig")
+                                  "/bin" "/etc" "/include" "/libexec"
+                                  "/sbin" "/share" "/var")))))))))))
+
+(define (multituned-package p)
+  (package/inherit p
+    (arguments
+     (substitute-keyword-arguments (package-arguments p)
+       ((#:phases phases #~%standard-phases)
+        (if (or (target-x86-64?)
+                (target-ppc64le?))
+          #~(modify-phases #$phases
+              (add-after 'install 'install-optimized-libraries
+                (lambda* (#:key inputs outputs #:allow-other-keys)
+                  (let ((hwcaps "/lib/glibc-hwcaps/"))
+                    (for-each
+                      (lambda (psabi)
+                        (copy-recursively
+                          (string-append
+                            (assoc-ref inputs (string-append
+                                                #$(package-name p) "-" psabi))
+                            hwcaps psabi)
+                          (string-append #$output hwcaps psabi)))
+                      #$(cond ((target-x86-64?)
+                               #~(list "x86-64-v2" "x86-64-v3" "x86-64-v4"))
+                              ((target-ppc64le?)
+                               #~(list "power9" "power10"))
+                              (#t #~'())))))))
+          phases))))
+    (inputs
+     (cond ((target-x86-64?)
+            (modify-inputs (package-inputs p)
+              (append (package-tuned-for-psabi p "x86-64-v2")
+                      (package-tuned-for-psabi p "x86-64-v3")
+                      (package-tuned-for-psabi p "x86-64-v4"))))
+           ((target-ppc64le?)
+            (modify-inputs (package-inputs p)
+              (append (package-tuned-for-psabi p "power9")
+                      (package-tuned-for-psabi p "power10"))))
+           (#t (package-inputs p))))
+    ;; With the addition of the psABIs this package should not be tuned.
+    (properties
+     (if (or (target-x86-64?)
+             (target-ppc64le?))
+         '((alist-delete 'tunable? (package-properties p)))
+         (package-properties p)))))
+
 (define (tunable-package? package)
   "Return true if package PACKAGE is \"tunable\"--i.e., if tuning it for the
 host CPU is worthwhile."

base-commit: 2a6d96425eea57dc6dd48a2bec16743046e32e06
-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#74253] [PATCH] transformations: Add multituned-package.

[Ludovic Courtès]

Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

> * guix/transformations.scm (package-tuned-for-psabi,
> multituned-package): New variables.
>
> Change-Id: I09ac7ae9fc2bcd9aa712b3c30fef807bc7d55895
> ---
>
> This allows wrapping a package definition in multituned-package, ie:
>
> (define-public opus
>  (multituned-package
>   (package
>    ...)))
>
> I'm not sure where to go with this patch from here. This will provide
> the psabi libraries for x86_64 and powerpc64le so they get most of the
> benefits from tuning for the architecture but without needing to specify
> which architecture to tune for.  It should also provide a nice boost for
> guix packs and docker images and the like.
>
> The downside with using this by default is the larger package size due
> to the extra versions of the libraries, and if it is used then the
> regular --tune is disabled for that package.
>
> I think adding it as a '--tune=generic' or '--tune=psabi' would be a
> nice way to use it.

Should that be a package transformation though?  Could we instead have a
build system trick or the ‘multituned-package’ procedure exposed so
build the package several times and fill in lib/glibc-hwcaps?

That way, packagers would explicitly choose this technique for select
packages, which would then no longer need the ‘tunable?’ property.

The question becomes: how would we choose which packages is eligible to
this technique as opposed to ‘--tune’?  Intuitively, I would use that
for general-purpose packages like ‘opus’, but keep ‘--tune’ for more
niche/scientific packages.

WDYT?

Thanks,
Ludo’.

[bug#74253] [PATCH] transformations: Add multituned-package.

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 3290 bytes --]

On Tue, Nov 12, 2024 at 11:31:57AM +0100, Ludovic Courtès wrote:
> Hi,
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > * guix/transformations.scm (package-tuned-for-psabi,
> > multituned-package): New variables.
> >
> > Change-Id: I09ac7ae9fc2bcd9aa712b3c30fef807bc7d55895
> > ---
> >
> > This allows wrapping a package definition in multituned-package, ie:
> >
> > (define-public opus
> >  (multituned-package
> >   (package
> >    ...)))
> >
> > I'm not sure where to go with this patch from here. This will provide
> > the psabi libraries for x86_64 and powerpc64le so they get most of the
> > benefits from tuning for the architecture but without needing to specify
> > which architecture to tune for.  It should also provide a nice boost for
> > guix packs and docker images and the like.
> >
> > The downside with using this by default is the larger package size due
> > to the extra versions of the libraries, and if it is used then the
> > regular --tune is disabled for that package.
> >
> > I think adding it as a '--tune=generic' or '--tune=psabi' would be a
> > nice way to use it.
> 
> Should that be a package transformation though?  Could we instead have a
> build system trick or the ‘multituned-package’ procedure exposed so
> build the package several times and fill in lib/glibc-hwcaps?

I figured (guix transformations) worked well since it was using an
existing transformation to generate the different variants.

I'm not sure what you mean by a build system trick, like build the
package multiple times in one like with x265?

This exposes the multituned-package procedure (looks like I mistyped in
the commit message) which takes a package and can be just added on top
of an existing package definition, like hidden-package can.

> That way, packagers would explicitly choose this technique for select
> packages, which would then no longer need the ‘tunable?’ property.

Something like a flag in the build-system like #:tests? is? Or like a
package property?

> The question becomes: how would we choose which packages is eligible to
> this technique as opposed to ‘--tune’?  Intuitively, I would use that
> for general-purpose packages like ‘opus’, but keep ‘--tune’ for more
> niche/scientific packages.

Unfortunately I think just like with choosing what to do with tunable?
we end up in the same spot deciding somewhat arbitrarily what to provide
the psabi options for and what not to.

> WDYT?
> 
> Thanks,
> Ludo’.

Looking more at the patch I've remembered that I've only taken care of
the gnu- and cmake- build-systems.  I was going to add about tuning the
package on other architectures, like aarch64, but it seems I fixed that
already.

If it were just the configure-flags and the post-installation removal of
files then I could see trying to make something that could be inserted
manually into specific packages, but the whole thing rests on the
tuned-package procedure actually producing libraries tuned for the
different psABIs.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#74253] [PATCH] transformations: Add multituned-package.

[Ludovic Courtès]

Hello,

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Tue, Nov 12, 2024 at 11:31:57AM +0100, Ludovic Courtès wrote:

[...]

>> Should that be a package transformation though?  Could we instead have a
>> build system trick or the ‘multituned-package’ procedure exposed so
>> build the package several times and fill in lib/glibc-hwcaps?
>
> I figured (guix transformations) worked well since it was using an
> existing transformation to generate the different variants.
>
> I'm not sure what you mean by a build system trick, like build the
> package multiple times in one like with x265?

Yes, that sort of thing, or basically what you have here but provided as
a build phase.

Now, it’s probably easier to implement it the way you did than as a
build phase, because how exactly to rebuild a package with the right
optimizations is very dependent on details about its build system.

> This exposes the multituned-package procedure (looks like I mistyped in
> the commit message) which takes a package and can be just added on top
> of an existing package definition, like hidden-package can.

Yes, but I think the key ideal with hwcaps is that, contrary to what
‘--tune’ does, we’d provide several optimized versions upfront.  In that
sense, we’d statically choose to “multi-tune” certain packages.

>> That way, packagers would explicitly choose this technique for select
>> packages, which would then no longer need the ‘tunable?’ property.
>
> Something like a flag in the build-system like #:tests? is? Or like a
> package property?

Either that, or write (define p' (multituned-package p)) etc.

The key point being: packagers would be the one deciding that.

So hmm, maybe the way forward is to keep the ‘multituned-package’, but
just not expose it as a package transformation.  Also, we might want to
move tuning support to (guix cpu-tuning) or similar, which is the module
that packagers would import (rather than (guix transformations)).

WDYT?

Ludo’.