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

Previous Next

Full log

Message #23 received at 60934 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: John Kehayias <john.kehayias <at> protonmail.com>, 60934 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 1/2] gnu: Add llvm-for-mesa.
Date: Mon, 23 Jan 2023 11:35:44 +0100

Hello!

Efraim Flashner <efraim <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.