[bug#60934] [CORE-UPDATES PATCH] gnu: mesa: Use smaller llvm backend.

[bug#60934] [CORE-UPDATES PATCH] gnu: mesa: Use smaller llvm backend.

[Efraim Flashner]

* gnu/packages/gl.scm (mesa)[inputs]: Replace llvm with llvm-for-mesa.
* gnu/packages/llvm.scm (llvm-for-mesa): New variable.
---

I wasn't able to tweak this to be able to build a cross compiled mesa. I
was able to build and run glxgears on core-updates with this patch
applied.

 gnu/packages/gl.scm   |  5 ++---
 gnu/packages/llvm.scm | 49 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/gl.scm b/gnu/packages/gl.scm
index dd62fac13e..bce2aead4c 100644
--- a/gnu/packages/gl.scm
+++ b/gnu/packages/gl.scm
@@ -5,7 +5,7 @@
 ;;; Copyright © 2014, 2015, 2016, 2017 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2016 Nikita <nikita@n0.is>
 ;;; Copyright © 2016, 2017, 2018, 2020, 2021 Ricardo Wurmus <rekado@elephly.net>
-;;; Copyright © 2017, 2018, 2019, 2021 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2017-2019, 2021, 2023 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2017 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2017, 2018, 2019 Rutger Helling <rhelling@mykolab.com>
 ;;; Copyright © 2018–2021 Tobias Geerinckx-Rice <me@tobias.gr>
@@ -298,8 +298,7 @@ (define-public mesa
            libxml2
            libxrandr
            libxvmc
-           ;; Note: update the 'clang' input of mesa-opencl when bumping this.
-           llvm
+           llvm-for-mesa
            wayland
            wayland-protocols))
     (native-inputs
diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index b46cb06443..2296647969 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -7,7 +7,7 @@
 ;;; Copyright © 2017 Roel Janssen <roel@gnu.org>
 ;;; Copyright © 2018–2022 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2018, 2019 Tobias Geerinckx-Rice <me@tobias.gr>
-;;; Copyright © 2018, 2021, 2022 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2018, 2021-2023 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
 ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
 ;;; Copyright © 2019 Rutger Helling <rhelling@mykolab.com>
@@ -1886,6 +1886,53 @@ (define-public emacs-clang-rename
 ;;; LLVM variants.
 ;;;
 
+(define-public llvm-for-mesa
+  ;; Note: update the 'clang' input of mesa-opencl when bumping this.
+  (let ((base-llvm llvm-13))
+    (package
+      (inherit base-llvm)
+      ;; If we can separate out the include directory we'd save another 23MB.
+      (outputs (list "out"))
+      (version (package-version base-llvm))
+      (arguments
+       (substitute-keyword-arguments (package-arguments base-llvm)
+         ((#:configure-flags _ ''())
+          #~(list
+              ;; AMDGPU is needed by the vulkan drivers.
+              #$(string-append "-DLLVM_TARGETS_TO_BUILD="
+                               (system->llvm-target) ";AMDGPU")
+              "-DLLVM_BUILD_TOOLS=NO"
+              "-DLLVM_BUILD_LLVM_DYLIB=YES"
+              "-DLLVM_LINK_LLVM_DYLIB=YES"
+              "-DLLVM_ENABLE_RTTI:BOOL=TRUE"))
+         ((#:phases phases '%standard-phases)
+          #~(modify-phases #$phases
+              (add-after 'install 'delete-static-libraries
+                (lambda* (#:key outputs #:allow-other-keys)
+                  (for-each delete-file
+                            (find-files (string-append
+                                          (assoc-ref outputs "out") "/lib")
+                                        "\\.a$"))))
+              ;; We don't need it for this version.
+              (replace 'install-opt-viewer
+                (lambda* (#:key outputs #:allow-other-keys)
+                  (let ((out (assoc-ref outputs "out")))
+                    (delete-file-recursively
+                      (string-append out "/share/opt-viewer")))))
+              (add-after 'install 'build-and-install-llvm-config
+                (lambda* (#:key outputs #:allow-other-keys)
+                  (let ((out (assoc-ref outputs "out")))
+                    (substitute*
+                      "tools/llvm-config/CMakeFiles/llvm-config.dir/link.txt"
+                      (((string-append "/tmp/guix-build-llvm-"
+                                       #$version ".drv-0/build/lib"))
+                       (string-append out "/lib")))
+                    (invoke "make" "llvm-config")
+                    (install-file "bin/llvm-config"
+                                  (string-append out "/bin")))))))))
+      (properties `((hidden? . #t)
+                    ,@(package-properties base-llvm))))))
+
 (define make-ocaml-llvm
   ;; Make it a memoizing procedure so its callers below don't end up defining
   ;; two equal-but-not-eq "ocaml-llvm" packages for the default LLVM.

base-commit: b8d684af76a41f5856b73640eb6f21940f214497
prerequisite-patch-id: 32cb914a2879991546162c9c87e371e7faf56c27
prerequisite-patch-id: 322468b8512360f3addd2e9788a3b53166fc3997
prerequisite-patch-id: 7b9e82394c1c3b8f7cb04ed14c6c17e7e6a19020
-- 
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#60934] [CORE-UPDATES PATCH] gnu: mesa: Use smaller llvm backend.

[John Kehayias via Guix-patches via]

Hi Efraim,

On Wed, Jan 18, 2023 at 04:49 PM, Efraim Flashner wrote:

> * gnu/packages/gl.scm (mesa)[inputs]: Replace llvm with llvm-for-mesa.
> * gnu/packages/llvm.scm (llvm-for-mesa): New variable.
> ---
>
> I wasn't able to tweak this to be able to build a cross compiled mesa. I
> was able to build and run glxgears on core-updates with this patch
> applied.
>

Nice!

Not sure if you already saw my message on guix-devel
https://lists.gnu.org/r/guix-devel/2023-01/msg00218.html but I'll just
repeat the summary: looks like latest mesa wants latest llvm (llvm-15 to
be exact) for current hardware support. I think this is the summary:
https://www.phoronix.com/news/LLVM-15-Branched

Have you tried with other versions of llvm?

I haven't checked what version of mesa is in core-updates or any pending
patches, but I can send an update to the latest version if needed.

I also haven't reviewed/tested this patch, but I do use current mesa via
another channel where I could try it out with this modification for
testing and get back to you.

Thanks for working on this!
John

[bug#60934] [CORE-UPDATES PATCH] gnu: mesa: Use smaller llvm backend.

[Efraim Flashner]

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

On Thu, Jan 19, 2023 at 03:14:46AM +0000, John Kehayias wrote:
> Hi Efraim,
> 
> On Wed, Jan 18, 2023 at 04:49 PM, Efraim Flashner wrote:
> 
> > * gnu/packages/gl.scm (mesa)[inputs]: Replace llvm with llvm-for-mesa.
> > * gnu/packages/llvm.scm (llvm-for-mesa): New variable.
> > ---
> >
> > I wasn't able to tweak this to be able to build a cross compiled mesa. I
> > was able to build and run glxgears on core-updates with this patch
> > applied.
> >
> 
> Nice!
> 
> Not sure if you already saw my message on guix-devel
> https://lists.gnu.org/r/guix-devel/2023-01/msg00218.html but I'll just
> repeat the summary: looks like latest mesa wants latest llvm (llvm-15 to
> be exact) for current hardware support. I think this is the summary:
> https://www.phoronix.com/news/LLVM-15-Branched
> 
> Have you tried with other versions of llvm?
> 
> I haven't checked what version of mesa is in core-updates or any pending
> patches, but I can send an update to the latest version if needed.
> 
> I also haven't reviewed/tested this patch, but I do use current mesa via
> another channel where I could try it out with this modification for
> testing and get back to you.
> 
> Thanks for working on this!
> John

I was going to start with llvm-15 but currently core-updates only has up
to llvm-14 and mesa is currently built there with llvm-13, so that's
what I used. I also ran into a number of cross compiling issues while
trying to test cross compiling on core-updates so I think I'll try
working it on master so it can be more easily tested, and then when it's
good I'll commit it to core-updates.  I'll also see about a
master->core-updates merge.

For the next version of the patch I think I'll copy the static libraries
to a static output instead of deleting them. I'm not sure exactly where
they'd be used, but at least in a different output they won't add to the
closure size if we need them and they're available if needed.

-- 
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#60934] [PATCH v2 0/2] llvm-for-mesa patch

[Efraim Flashner]

I've rebased the patch on top of master which should make testing it
easier. I tested it with glxgears from mesa-utils.

The first patch is just the patch backported.

The second patch makes some small adjustments so that the version of
mesa in master can be built with llvm-for-mesa.

When the patch is ready for inclusion we have a choice between merging
master into core-updates and then applying it, so we can keep llvm-15 as
base-llvm, or I can commit it to master with a TODO that it should
replace the llvm input in mesa in core-updates.

Efraim Flashner (2):
  gnu: Add llvm-for-mesa.
  HELPER PATCH

 gnu/packages/gl.scm   |  2 +-
 gnu/packages/llvm.scm | 53 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)


base-commit: 26bb41d6d503a62e86b853774f6d8313abef2123
-- 
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#60934] [PATCH v2 1/2] gnu: Add llvm-for-mesa.

[Efraim Flashner]

* gnu/packages/llvm.scm (llvm-for-mesa): New variable.
---
 gnu/packages/llvm.scm | 49 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 58bd91d7be..8379b23fa5 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -2070,6 +2070,55 @@ (define-public emacs-clang-rename
 ;;; LLVM variants.
 ;;;
 
+(define-public llvm-for-mesa
+  ;; Note: update the 'clang' input of mesa-opencl when bumping this.
+  (let ((base-llvm llvm-15))
+    (package
+      (inherit base-llvm)
+      (arguments
+       (substitute-keyword-arguments (package-arguments base-llvm)
+         ((#:modules modules '((guix build cmake-build-system)
+                               (guix build utils)))
+          `((ice-9 regex)
+            (srfi srfi-1)
+            (srfi srfi-26)
+            ,@modules))
+         ((#:configure-flags cf ''())
+          #~(cons*
+              ;; AMDGPU is needed by the vulkan drivers.
+              #$(string-append "-DLLVM_TARGETS_TO_BUILD="
+                               (system->llvm-target) ";AMDGPU")
+              ;; Tools and utils decrease the output by ~100 MiB.
+              "-DLLVM_BUILD_TOOLS=NO"
+              (remove (cut string-match
+                           "-DLLVM_(TARGETS_TO_BUILD|INSTALL_UTILS).*" <>)
+                      #$cf)))
+         ((#:phases phases '%standard-phases)
+          #~(modify-phases #$phases
+              (add-after 'install 'delete-static-libraries
+                ;; If these are just relocated then llvm-config can't find them.
+                (lambda* (#:key outputs #:allow-other-keys)
+                  (for-each delete-file
+                            (find-files (string-append
+                                          (assoc-ref outputs "out") "/lib")
+                                        "\\.a$"))))
+              (add-after 'install 'build-and-install-llvm-config
+                (lambda* (#:key outputs #:allow-other-keys)
+                  (let ((out (assoc-ref outputs "out")))
+                    (substitute*
+                      "tools/llvm-config/CMakeFiles/llvm-config.dir/link.txt"
+                      (((string-append "/tmp/guix-build-llvm-"
+                                       #$(package-version base-llvm)
+                                       ".drv-0/build/lib"))
+                       (string-append out "/lib")))
+                    (invoke "make" "llvm-config")
+                    (install-file "bin/llvm-config"
+                                  (string-append out "/bin")))))))))
+      ;; We don't override the name so we hide it so we don't have package
+      ;; ambiguities from the CLI.
+      (properties `((hidden? . #t)
+                    ,@(package-properties base-llvm))))))
+
 (define make-ocaml-llvm
   ;; Make it a memoizing procedure so its callers below don't end up defining
   ;; two equal-but-not-eq "ocaml-llvm" packages for the default LLVM.
-- 
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#60934] [PATCH v2 2/2] HELPER PATCH

[Efraim Flashner]

This patch is to allow testing llvm-for-mesa while allowing the actual
patch to be cleanly shown.

* gnu/packages/gl.scm (mesa)[inputs]: Replace llvm-11 with llvm-for-mesa.
* gnu/packages/llvm.scm (llvm-for-mesa): Inherit from llvm-14.
[arguments]: Inherit package arguments from llvm-15.
---
 gnu/packages/gl.scm   | 2 +-
 gnu/packages/llvm.scm | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/gl.scm b/gnu/packages/gl.scm
index 01ab6135a4..084c14dd2f 100644
--- a/gnu/packages/gl.scm
+++ b/gnu/packages/gl.scm
@@ -304,7 +304,7 @@ (define-public mesa
                    wayland-protocols)
              ;; TODO: Resort alphabetically.
              ;; Note: update the 'clang' input of mesa-opencl when bumping this.
-             (list llvm-11)))
+             (list llvm-for-mesa)))
     (native-inputs
      (append (list bison
                    flex
diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 8379b23fa5..23332dd8e2 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -2072,11 +2072,15 @@ (define-public emacs-clang-rename
 
 (define-public llvm-for-mesa
   ;; Note: update the 'clang' input of mesa-opencl when bumping this.
-  (let ((base-llvm llvm-15))
+  ;; llvm-15 is missing include/llvm-c/Transforms/Coroutines.h, needed by mesa-21.3.
+  ;; Therefore we inherit from llvm-14 but use the package arguments from llvm-15.
+  ;; This is an unsupported configuration of configure-flags for cross-building but
+  ;; works for mesa.
+  (let ((base-llvm llvm-14))
     (package
       (inherit base-llvm)
       (arguments
-       (substitute-keyword-arguments (package-arguments base-llvm)
+       (substitute-keyword-arguments (package-arguments llvm-15)
          ((#:modules modules '((guix build cmake-build-system)
                                (guix build utils)))
           `((ice-9 regex)
-- 
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#60934] [PATCH v2 1/2] gnu: Add llvm-for-mesa.

[Ludovic Courtès]

Hello!

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

> * gnu/packages/llvm.scm (llvm-for-mesa): New variable.

Yay for reduced closures!!

> +(define-public llvm-for-mesa

Maybe add a line saying that this is a slim variant specifically
tailored for Mesa, that takes X% of the size of the default LLVM.

> +  ;; Note: update the 'clang' input of mesa-opencl when bumping this.
> +  (let ((base-llvm llvm-15))
> +    (package
> +      (inherit base-llvm)

Add (name "llvm-for-mesa") ?

> +      (arguments
> +       (substitute-keyword-arguments (package-arguments base-llvm)
> +         ((#:modules modules '((guix build cmake-build-system)
> +                               (guix build utils)))
> +          `((ice-9 regex)
> +            (srfi srfi-1)
> +            (srfi srfi-26)
> +            ,@modules))
> +         ((#:configure-flags cf ''())
> +          #~(cons*
> +              ;; AMDGPU is needed by the vulkan drivers.
> +              #$(string-append "-DLLVM_TARGETS_TO_BUILD="
> +                               (system->llvm-target) ";AMDGPU")

So the result is two build only two backends, for example x86_64 and
AMDGPU, right?

> +         ((#:phases phases '%standard-phases)
> +          #~(modify-phases #$phases
> +              (add-after 'install 'delete-static-libraries
> +                ;; If these are just relocated then llvm-config can't find them.
> +                (lambda* (#:key outputs #:allow-other-keys)
> +                  (for-each delete-file
> +                            (find-files (string-append
> +                                          (assoc-ref outputs "out") "/lib")
> +                                        "\\.a$"))))

Should we pass -DDISABLE_STATIC=ON or whatever it’s called instead?

> +              (add-after 'install 'build-and-install-llvm-config
> +                (lambda* (#:key outputs #:allow-other-keys)

Why do we need this extra phase compared to ‘llvm’?  Please add a
comment.  :-)

> +                  (let ((out (assoc-ref outputs "out")))
> +                    (substitute*
> +                      "tools/llvm-config/CMakeFiles/llvm-config.dir/link.txt"
> +                      (((string-append "/tmp/guix-build-llvm-"
> +                                       #$(package-version base-llvm)
> +                                       ".drv-0/build/lib"))
> +                       (string-append out "/lib")))

The non-literal pattern here, and that it explicitly refers to the build
directory, is not great.

I believe you can use (string-append (getcwd) "/lib") as the pattern,
fixing the second problem.  Not sure about the first one.

Thank you!

Ludo’.

[bug#60934] [PATCH v2 1/2] gnu: Add llvm-for-mesa.

[Efraim Flashner]

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

On Mon, Jan 23, 2023 at 11:35:44AM +0100, Ludovic Courtès wrote:
> Hello!
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > * gnu/packages/llvm.scm (llvm-for-mesa): New variable.
> 
> Yay for reduced closures!!
> 
> > +(define-public llvm-for-mesa
> 
> Maybe add a line saying that this is a slim variant specifically
> tailored for Mesa, that takes X% of the size of the default LLVM.

Will do.

> > +  ;; Note: update the 'clang' input of mesa-opencl when bumping this.
> > +  (let ((base-llvm llvm-15))
> > +    (package
> > +      (inherit base-llvm)
> 
> Add (name "llvm-for-mesa") ?

I don't have a strong preference. By leaving it as 'llvm' we make it
more easily substitutable by any other build of llvm since they'd have
the same name length. Having it as 'llvm-for-mesa' makes it really clear
what it is.

> > +      (arguments
> > +       (substitute-keyword-arguments (package-arguments base-llvm)
> > +         ((#:modules modules '((guix build cmake-build-system)
> > +                               (guix build utils)))
> > +          `((ice-9 regex)
> > +            (srfi srfi-1)
> > +            (srfi srfi-26)
> > +            ,@modules))
> > +         ((#:configure-flags cf ''())
> > +          #~(cons*
> > +              ;; AMDGPU is needed by the vulkan drivers.
> > +              #$(string-append "-DLLVM_TARGETS_TO_BUILD="
> > +                               (system->llvm-target) ";AMDGPU")
> 
> So the result is two build only two backends, for example x86_64 and
> AMDGPU, right?

That's right. I tried with just (system->llvm-target) but AMDGPU was
needed for the vulkan drivers.

> > +         ((#:phases phases '%standard-phases)
> > +          #~(modify-phases #$phases
> > +              (add-after 'install 'delete-static-libraries
> > +                ;; If these are just relocated then llvm-config can't find them.
> > +                (lambda* (#:key outputs #:allow-other-keys)
> > +                  (for-each delete-file
> > +                            (find-files (string-append
> > +                                          (assoc-ref outputs "out") "/lib")
> > +                                        "\\.a$"))))
> 
> Should we pass -DDISABLE_STATIC=ON or whatever it’s called instead?

It turns out that static is the default, and we override it in most of
the llvm packages to build shared libraries instead. If we disable the
static ones too then it errors out and says it has nothing to build.

> > +              (add-after 'install 'build-and-install-llvm-config
> > +                (lambda* (#:key outputs #:allow-other-keys)
> 
> Why do we need this extra phase compared to ‘llvm’?  Please add a
> comment.  :-)

Fair enough :) It turns out that *everyone* expects llvm-config to
exist, and doing it this way allows us to only build llvm-config, not
all the tools/utilities.

> > +                  (let ((out (assoc-ref outputs "out")))
> > +                    (substitute*
> > +                      "tools/llvm-config/CMakeFiles/llvm-config.dir/link.txt"
> > +                      (((string-append "/tmp/guix-build-llvm-"
> > +                                       #$(package-version base-llvm)
> > +                                       ".drv-0/build/lib"))
> > +                       (string-append out "/lib")))
> 
> The non-literal pattern here, and that it explicitly refers to the build
> directory, is not great.
> 
> I believe you can use (string-append (getcwd) "/lib") as the pattern,
> fixing the second problem.  Not sure about the first one.

I suppose it would be (string-append (getcwd) "/bin/lib") to fix both. I
wasn't able to find a good way to get llvm-config to be configured
correctly by the build system without also having it install all the
utilities. When I looked in the build directory after a RUNPATH failure
it had that directory as the one to link to for the libraries.

> Thank you!

This one's been bothering everyone for years :)

-- 
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#60934: [PATCH v2 1/2] gnu: Add llvm-for-mesa.

[Efraim Flashner]

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

master merged into core-updates and this patch pushed!

-- 
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 --]