[PATCH v2] lib/meson.build: allow build IGT with link time optimization

[Lucas De Marchi]

On Fri, Jan 10, 2025 at 08:58:39AM +0100, Hajda, Andrzej wrote:
>
>W dniu 09.01.2025 o 18:29, Lucas De Marchi pisze:
>>On Thu, Jan 09, 2025 at 05:26:30PM +0100, Andrzej Hajda wrote:
>>>Some distributions use -flto gcc flag to build IGT. With this flag
>>>enabled variables declared with section attribute do not appear in
>>>correct section in .o files. Since iga64 assembly helper script relies
>>>on it we need to use -ffat-lto-objects in case of files containing
>>>iga64 assembly. According to documentation this option is effective
>>>only in case lto is in use, so it should not affect non-lto builds.
>>>
>>>Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/165
>>>Signed-off-by: Andrzej Hajda <andrzej.hajda at intel.com>
>>
>>is this about
>>
>>        static const char t[] __attribute__ 
>>((section(".iga64_assembly"), used)) =\
>>?
>>
>>it's missing a "retain" attribute to be able to link with LLD.
>>I fixed a similar bug in kmod recently, but that  was due to linking with
>>LLD instead of using or not lto. See 
>>https://github.com/kmod-project/kmod/pull/274
>>
>Yes this is about this line, but I think we have here slightly 
>different issues.
>
>As I understand in kmod-project you want to keep the symbol in final 
>executable/so_library/whatever.

yep

>
>In case of igt we want to have access to ".iga64_assembly" section 
>after compile phase (ie see it in .o files), we do not care about 
>final executable.

oh... ok. The scripts/generate_iga64_codes script will parse the
intermediate files, not the final lib.

>
>With LTO enabled the structure of .o files drastically changes - every 
>function/variable in compile phase goes to separate section 
>(.gnu.lto_*), information about final sections is kept somewhere in 
>private metadata.
>
>With compiler option '-ffat-lto-objects' .o file contains both 
>structures of sections - classic .text/.data/.iga64_assembly and lto 
>related .gnu.lto_* and this is what we need in case of igt.

is this true for both gcc/clang? With clang it seems it seems it's even
worse:

$ file build-lto-clang/lib/libigt-gpgpu_fill_c.a.p/gpgpu_fill.c.o
build-lto-clang/lib/libigt-gpgpu_fill_c.a.p/gpgpu_fill.c.o: LLVM IR bitcode

$ readelf -t build-lto-clang/lib/libigt-gpgpu_fill_c.a.p/gpgpu_fill.c.o
readelf: Error: This is a LLVM bitcode file - try using llvm-bcanalyzer

This seems to fix it for gcc and we may handle the above as a
separate case. For fixing this separate case, maybe we should stop
inspecting the .o, add a retain attribute, inspect the final
lib/binary and use objcopy --add-symbol to add the new symbols (or the
linker again to link one additional .o at the end). Anyway, I'm
digressing.


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>


thanks
Lucas De Marchi

>
>
>Regards
>
>Andrzej
>
>
>>Are you sure this is related to LTO and not the different linker?
>>$ grep -r -e '\.iga64_assembly' build-lto-clang/
>>grep: 
>>build-lto-clang/lib/libigt-gpgpu_shader_c.a.p/gpgpu_shader.c.o: 
>>binary file matches
>>grep: build-lto-clang/lib/libigt.so.0: binary file matches
>>grep: build-lto-clang/lib/libigt-gpgpu_fill_c.a.p/gpgpu_fill.c.o: 
>>binary file matches
>>
>>
>>$ readelf -t build-lto-clang/lib/libigt.so.0 | grep -A3 -e 
>>.iga64_assembly
>>  [10] .iga64_assembly
>>       PROGBITS         00000000000657d0  00000000000657d0  0
>>       0000000000002c65 0000000000000000  0                 16
>>       [0000000000000002]: ALLOC
>>
>>So it has a section with that name of size 0x2c65
>>
>>
>>Lucas De Marchi
>>
>>>---
>>>This patch 'fixes' building IGT with link-time-optimization. I am not
>>>sure it is or should be supported by IGT, but build scripts can be
>>>modified to allow it for testing.
>>>Grepping internet shows at least Gentoo and Arch Linux builds IGT with
>>>LTO, so I guess it is working for them.
>>>---
>>>Changes in v2:
>>>- added Closes tag
>>>- Link to v1: https://lore.kernel.org/r/20241216-fix-lto-v1-1-1280b17dc70f@intel.com
>>>---
>>>lib/meson.build | 2 +-
>>>1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/lib/meson.build b/lib/meson.build
>>>index 1704ed1e1c17..9fffdd3c6218 100644
>>>--- a/lib/meson.build
>>>+++ b/lib/meson.build
>>>@@ -240,7 +240,7 @@ foreach f: lib_sources
>>>        '-DIGT_DATADIR="@0@"'.format(join_paths(prefix, datadir)),
>>>        '-DIGT_SRCDIR="@0@"'.format(srcdir),
>>>        '-DIGT_LOG_DOMAIN="@0@"'.format(f.split('.')[0]),
>>>-    ])
>>>+    ] + (iga64_assembly_sources.contains(f) ? [ 
>>>'-ffat-lto-objects' ] : []))
>>>
>>>    lib_intermediates += lib
>>>    if iga64_assembly_sources.contains(f)
>>>
>>>---
>>>base-commit: ee7a3ac616f55f6ed1b959ff951237099bda86d8
>>>change-id: 20241216-fix-lto-8d3bc1488c6a
>>>
>>>Best regards,
>>>-- 
>>>Andrzej Hajda <andrzej.hajda at intel.com>
>>>