Skip to contentSkip to conversation

kbuild: Allow to reduce the number of suffixes for clang thin-lto build

PATCH Superseded Yonghong Song 3 participants 8 messages Mar 6 – Mar 7

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kbuild: Allow to reduce the number of suffixes for clang thin-lto build

The current clang thin-lto build often produces lots of symbols with suffix. The following is a partial list of such function call symbols:

    ...
    ethnl_module_fw_flash_ntf.llvm.7631589765585346066
    __nf_conntrack_alloc.llvm.6438426151906658917
    tcp_can_early_drop.llvm.11937612064648250727
    tcp_print_conntrack.llvm.11937612064648250727
    ...

In my particular build with current bpf-next, the number of '*.llvm.<hash>' function calls is 1212. Such symbols make kernel live patching difficult since
- a minor code change will change the hash and then the '*.llvm.<hash>' symbol becomes another one with a different hash or no hash, and
- a previous source-level symbol may become an one with suffix after live patching code.

In [1], Song Liu suggested to reduce the number of '*.llvm.<hash>' functions to make live patch easier. In respond of this, I implemented this in llvm ([2]). The same thin-lto build with [2] only has two symbols with suffix:

    m_stop.llvm.14460341347352036579
    m_next.llvm.14460341347352036579

This should make live patch much easier.

To support suffix symbol reduction, a new config
LTO_CLANG_THIN_SUFFIX_REDUCTION
is introduced and the config depends on thin-lto and llvm23 or higher.

Two lld flags are necessary to enable this feature in kernel:
- Flag '--lto-whole-program-visibility' is needed as it ensures that all modules are available in the same process, which is true for kernel at thin-lto lld.
- Flag '-mllvm -always-rename-promoted-locals=false' is needed to enable suffix reduction. Currently in llvm [1], only process mode is supported. There is another distributed mode (across different processes or even different machines) which is not supported yet ([2]).

[1] https://lpc.events/event/19/contributions/2212
[2] https://github.com/llvm/llvm-project/pull/178587

---
 Makefile     |  3 +++
 arch/Kconfig | 15 +++++++++++++++
 2 files changed, 18 insertions(+)
Makefile +3 -0
index e944c6e71e81..9d6033595615 100644
@@ -1034,6 +1034,9 @@ endif
 ifdef CONFIG_LTO_CLANG
 ifdef CONFIG_LTO_CLANG_THIN
 CC_FLAGS_LTO	:= -flto=thin -fsplit-lto-unit
+ifdef CONFIG_LTO_CLANG_THIN_SUFFIX_REDUCTION
+KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false
+endif
 else
 CC_FLAGS_LTO	:= -flto
 endif
arch/Kconfig +15 -0
index 102ddbd4298e..e1db64a3284e 100644
@@ -861,8 +861,23 @@ config LTO_CLANG_THIN
 	    https://clang.llvm.org/docs/ThinLTO.html
 
 	  If unsure, say Y.
+
 endchoice
 
+config LTO_CLANG_THIN_SUFFIX_REDUCTION
+	bool "Clang ThinLTO Suffix Reduction (EXPERIMENTAL)"
+	depends on LTO_CLANG_THIN
+	depends on CLANG_VERSION >= 230000
+	default y
+	help
+	  This option allows to reduce the number of symbols with
+	  '.llvm.<hash' suffixes. This can help KLP (kernel living
+	  patch) as symbol name can stay stable in most cases.
+
+	  See https://github.com/llvm/llvm-project/pull/178587
+
+	  If unsure, say N.
+
 config ARCH_SUPPORTS_AUTOFDO_CLANG
 	bool
 
Signature · 1 line
2.47.3
maintainer
#1 lore raw
Quoted 76 lines · Yonghong Song

The current clang thin-lto build often produces lots of symbols with suffix. The following is a partial list of such function call symbols:

    ...
    ethnl_module_fw_flash_ntf.llvm.7631589765585346066
    __nf_conntrack_alloc.llvm.6438426151906658917
    tcp_can_early_drop.llvm.11937612064648250727
    tcp_print_conntrack.llvm.11937612064648250727
    ...

In my particular build with current bpf-next, the number of '*.llvm.<hash>' function calls is 1212. Such symbols make kernel live patching difficult since
- a minor code change will change the hash and then the '*.llvm.<hash>' symbol becomes another one with a different hash or no hash, and
- a previous source-level symbol may become an one with suffix after live patching code.

In [1], Song Liu suggested to reduce the number of '*.llvm.<hash>' functions to make live patch easier. In respond of this, I implemented this in llvm ([2]). The same thin-lto build with [2] only has two symbols with suffix:

    m_stop.llvm.14460341347352036579
    m_next.llvm.14460341347352036579

This should make live patch much easier.

To support suffix symbol reduction, a new config
LTO_CLANG_THIN_SUFFIX_REDUCTION
is introduced and the config depends on thin-lto and llvm23 or higher.

Two lld flags are necessary to enable this feature in kernel:
- Flag '--lto-whole-program-visibility' is needed as it ensures that all modules are available in the same process, which is true for kernel at thin-lto lld.
- Flag '-mllvm -always-rename-promoted-locals=false' is needed to enable suffix reduction. Currently in llvm [1], only process mode is supported. There is another distributed mode (across different processes or even different machines) which is not supported yet ([2]).

[1] https://lpc.events/event/19/contributions/2212
[2] https://github.com/llvm/llvm-project/pull/178587

---
 Makefile     |  3 +++
 arch/Kconfig | 15 +++++++++++++++
 2 files changed, 18 insertions(+)
Makefile +3 -0
index e944c6e71e81..9d6033595615 100644
@@ -1034,6 +1034,9 @@ endif
 ifdef CONFIG_LTO_CLANG
 ifdef CONFIG_LTO_CLANG_THIN
 CC_FLAGS_LTO	:= -flto=thin -fsplit-lto-unit
+ifdef CONFIG_LTO_CLANG_THIN_SUFFIX_REDUCTION
+KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false
+endif
 else
 CC_FLAGS_LTO	:= -flto
 endif
arch/Kconfig +5 -0
index 102ddbd4298e..e1db64a3284e 100644
@@ -861,8 +861,23 @@ config LTO_CLANG_THIN
 	    https://clang.llvm.org/docs/ThinLTO.html
 
 	  If unsure, say Y.
+
 endchoice
 
+config LTO_CLANG_THIN_SUFFIX_REDUCTION
+	bool "Clang ThinLTO Suffix Reduction (EXPERIMENTAL)"
+	depends on LTO_CLANG_THIN
+	depends on CLANG_VERSION >= 230000

arch/Kconfig config LTO_CLANG_THIN · line 880

Quoted 10 lines
arch/Kconfig +10 -0
+	default y
+	help
+	  This option allows to reduce the number of symbols with
+	  '.llvm.<hash' suffixes. This can help KLP (kernel living
+	  patch) as symbol name can stay stable in most cases.
+
+	  See https://github.com/llvm/llvm-project/pull/178587
+
+	  If unsure, say N.
+

Thanks! Would there be any downsides to enabling this feature unconditionally in the kernel when the compiler supports it?

Signature · 1 line
Josh
#2 lore raw
Quoted 73 lines · Josh Poimboeuf
Quoted 72 lines · Yonghong Song

The current clang thin-lto build often produces lots of symbols with suffix. The following is a partial list of such function call symbols:

     ...
     ethnl_module_fw_flash_ntf.llvm.7631589765585346066
     __nf_conntrack_alloc.llvm.6438426151906658917
     tcp_can_early_drop.llvm.11937612064648250727
     tcp_print_conntrack.llvm.11937612064648250727
     ...

In my particular build with current bpf-next, the number of '*.llvm.<hash>' function calls is 1212. Such symbols make kernel live patching difficult since
- a minor code change will change the hash and then the '*.llvm.<hash>' symbol becomes another one with a different hash or no hash, and
- a previous source-level symbol may become an one with suffix after live patching code.

In [1], Song Liu suggested to reduce the number of '*.llvm.<hash>' functions to make live patch easier. In respond of this, I implemented this in llvm ([2]). The same thin-lto build with [2] only has two symbols with suffix:

     m_stop.llvm.14460341347352036579
     m_next.llvm.14460341347352036579

This should make live patch much easier.

To support suffix symbol reduction, a new config
LTO_CLANG_THIN_SUFFIX_REDUCTION
is introduced and the config depends on thin-lto and llvm23 or higher.

Two lld flags are necessary to enable this feature in kernel:
- Flag '--lto-whole-program-visibility' is needed as it ensures that all modules are available in the same process, which is true for kernel at thin-lto lld.
- Flag '-mllvm -always-rename-promoted-locals=false' is needed to enable suffix reduction. Currently in llvm [1], only process mode is supported. There is another distributed mode (across different processes or even different machines) which is not supported yet ([2]).

   [1] https://lpc.events/event/19/contributions/2212
   [2] https://github.com/llvm/llvm-project/pull/178587
---
  Makefile     |  3 +++
  arch/Kconfig | 15 +++++++++++++++
  2 files changed, 18 insertions(+)
Makefile +3 -0
index e944c6e71e81..9d6033595615 100644
@@ -1034,6 +1034,9 @@ endif
  ifdef CONFIG_LTO_CLANG
  ifdef CONFIG_LTO_CLANG_THIN
  CC_FLAGS_LTO	:= -flto=thin -fsplit-lto-unit
+ifdef CONFIG_LTO_CLANG_THIN_SUFFIX_REDUCTION
+KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false
+endif
  else
  CC_FLAGS_LTO	:= -flto
  endif
arch/Kconfig +1 -0
index 102ddbd4298e..e1db64a3284e 100644
@@ -861,8 +861,23 @@ config LTO_CLANG_THIN
  	    https://clang.llvm.org/docs/ThinLTO.html
 
  	  If unsure, say Y.
+
  endchoice
 
Quoted 16 lines
Quoted 4 lines
arch/Kconfig +4 -0
+config LTO_CLANG_THIN_SUFFIX_REDUCTION
+	bool "Clang ThinLTO Suffix Reduction (EXPERIMENTAL)"
+	depends on LTO_CLANG_THIN
+	depends on CLANG_VERSION >= 230000

arch/Kconfig config LTO_CLANG_THIN · line 880

Quoted 10 lines
arch/Kconfig +10 -0
+	default y
+	help
+	  This option allows to reduce the number of symbols with
+	  '.llvm.<hash' suffixes. This can help KLP (kernel living
+	  patch) as symbol name can stay stable in most cases.
+
+	  See https://github.com/llvm/llvm-project/pull/178587
+
+	  If unsure, say N.
+

Thanks! Would there be any downsides to enabling this feature unconditionally in the kernel when the compiler supports it?

The only downside is for the following case:

C file: static function foo()
Asm file: global function foo()

The thin-lto will collect all C files and with the above llvm patch, the static function foo() may be promoted to global function foo() if there is no other pre-existing global function foo() in all C files.

In such cases, there will be a conflict since
there are two global function foo() (one from C file, another from Asm file). In such cases, the build will fail.

How do you think we could hit such issues in linux kernel? Maybe should have default no for the new config?

I think the chance should be very low. The following is a grab for x86 for global symbols in asm code:

[~/work/others/linux/arch/x86 (master)]$ egrep -r globl

egrep: warning: egrep is obsolescent; using grep -E
boot/compressed/mkpiggy.c:      printf(".globl z_input_len\n");
boot/compressed/mkpiggy.c:      printf(".globl z_output_len\n");
boot/compressed/mkpiggy.c:      printf(".globl input_data, input_data_end\n");
boot/compressed/mkpiggy.c:      printf(".globl input_len\n");
boot/compressed/mkpiggy.c:      printf(".globl output_len\n");
boot/compressed/head_64.S:      .globl  verify_cpu
boot/bioscall.S:        .globl  intcall
boot/header.S:  .globl  pecompat_fstart
boot/header.S:  .globl  sentinel
boot/header.S:  .globl  hdr
boot/header.S:  .globl  _start
boot/header.S:          .globl realmode_swtch
boot/header.S:  .globl  die
entry/vdso/vdso32/sigreturn.S:  .globl __kernel_sigreturn
entry/vdso/vdso32/sigreturn.S:  .globl __kernel_rt_sigreturn
entry/vdso/vdso32/system_call.S:        .globl __kernel_vsyscall
entry/vsyscall/vsyscall_emu_64.S:       .globl __vsyscall_page
entry/entry_32.S:       .globl __irqentry_text_start
entry/entry_32.S:       .globl __irqentry_text_end
entry/entry_64.S:       .globl __irqentry_text_start
entry/entry_64.S:       .globl __irqentry_text_end
include/asm/paravirt_types.h:       ".globl " PV_THUNK_NAME(func) ";"                           \
include/asm/static_call.h:          ".globl " STATIC_CALL_TRAMP_STR(name) "             \n"     \
kernel/cpu/amd.c:       ".globl vide\n"

kernel/ftrace_32.S:.globl ftrace_call
kernel/ftrace_32.S:.globl ftrace_graph_call
kernel/ftrace_32.S:.globl return_to_handler
kernel/relocate_kernel_32.S: .globl kexec_control_code_size
kernel/head_32.S:.globl initial_pg_pmd
kernel/head_32.S:.globl initial_page_table
kernel/head_32.S:.globl swapper_pg_dir
kernel/head_32.S:.globl empty_zero_page
lib/error-inject.c: ".globl just_return_func\n"
math-emu/reg_round.S:.globl fpu_reg_round
math-emu/reg_round.S:.globl fpu_Arith_exit

purgatory/kexec-purgatory.S:    .globl  kexec_purgatory
purgatory/kexec-purgatory.S:    .globl  kexec_purgatory_size
um/setjmp_32.S: .globl kernel_setjmp
um/setjmp_32.S: .globl kernel_longjmp
um/setjmp_64.S: .globl kernel_setjmp
um/setjmp_64.S: .globl kernel_longjmp
xen/xen-head.S: ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          .globl xen_elfnote_entry;

[~/work/others/linux/arch/x86 (master)]$

Maybe we could collect all global symbols in asm codes before lld, and then we add an option in lld to feed those global symbols (with a file?), then we can be sure there won't be any conflict?

maintainer
#3 lore raw
Quoted 104 lines · Yonghong Song
Quoted 79 lines · Josh Poimboeuf
Quoted 78 lines · Yonghong Song

The current clang thin-lto build often produces lots of symbols with suffix. The following is a partial list of such function call symbols:

     ...
     ethnl_module_fw_flash_ntf.llvm.7631589765585346066
     __nf_conntrack_alloc.llvm.6438426151906658917
     tcp_can_early_drop.llvm.11937612064648250727
     tcp_print_conntrack.llvm.11937612064648250727
     ...

In my particular build with current bpf-next, the number of '*.llvm.<hash>' function calls is 1212. Such symbols make kernel live patching difficult since
- a minor code change will change the hash and then the '*.llvm.<hash>' symbol becomes another one with a different hash or no hash, and
- a previous source-level symbol may become an one with suffix after live patching code.

In [1], Song Liu suggested to reduce the number of '*.llvm.<hash>' functions to make live patch easier. In respond of this, I implemented this in llvm ([2]). The same thin-lto build with [2] only has two symbols with suffix:

     m_stop.llvm.14460341347352036579
     m_next.llvm.14460341347352036579

This should make live patch much easier.

To support suffix symbol reduction, a new config
LTO_CLANG_THIN_SUFFIX_REDUCTION
is introduced and the config depends on thin-lto and llvm23 or higher.

Two lld flags are necessary to enable this feature in kernel:
- Flag '--lto-whole-program-visibility' is needed as it ensures that all modules are available in the same process, which is true for kernel at thin-lto lld.
- Flag '-mllvm -always-rename-promoted-locals=false' is needed to enable suffix reduction. Currently in llvm [1], only process mode is supported. There is another distributed mode (across different processes or even different machines) which is not supported yet ([2]).

   [1] https://lpc.events/event/19/contributions/2212
   [2] https://github.com/llvm/llvm-project/pull/178587
---
  Makefile     |  3 +++
  arch/Kconfig | 15 +++++++++++++++
  2 files changed, 18 insertions(+)
Makefile +3 -0
index e944c6e71e81..9d6033595615 100644
@@ -1034,6 +1034,9 @@ endif
  ifdef CONFIG_LTO_CLANG
  ifdef CONFIG_LTO_CLANG_THIN
  CC_FLAGS_LTO	:= -flto=thin -fsplit-lto-unit
+ifdef CONFIG_LTO_CLANG_THIN_SUFFIX_REDUCTION
+KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false
+endif
  else
  CC_FLAGS_LTO	:= -flto
  endif
arch/Kconfig +9 -0
index 102ddbd4298e..e1db64a3284e 100644
@@ -861,8 +861,23 @@ config LTO_CLANG_THIN
  	    https://clang.llvm.org/docs/ThinLTO.html
  	  If unsure, say Y.
+
  endchoice
+config LTO_CLANG_THIN_SUFFIX_REDUCTION
+	bool "Clang ThinLTO Suffix Reduction (EXPERIMENTAL)"
+	depends on LTO_CLANG_THIN
+	depends on CLANG_VERSION >= 230000
+	default y
+	help
+	  This option allows to reduce the number of symbols with
+	  '.llvm.<hash' suffixes. This can help KLP (kernel living
Quoted 8 lines

arch/Kconfig config LTO_CLANG_THIN · line 880

Quoted 6 lines
arch/Kconfig +6 -0
+	  patch) as symbol name can stay stable in most cases.
+
+	  See https://github.com/llvm/llvm-project/pull/178587
+
+	  If unsure, say N.
+

Thanks! Would there be any downsides to enabling this feature unconditionally in the kernel when the compiler supports it?

The only downside is for the following case:

C file: static function foo()
Asm file: global function foo()

The thin-lto will collect all C files and with the above llvm patch, the static function foo() may be promoted to global function foo() if there is no other pre-existing global function foo() in all C files.

In such cases, there will be a conflict since
there are two global function foo() (one from C file, another from Asm file). In such cases, the build will fail.

Quoted 7 lines

How do you think we could hit such issues in linux kernel? Maybe should have default no for the new config?

I think the chance should be very low. The following is a grab for x86 for global symbols in asm code:

[~/work/others/linux/arch/x86 (master)]$ egrep -r globl

There are actually quite a bit more than that, see SYM_CODE_START:

  $ git grep 'SYM_CODE_START(' |wc -l
  169

But still, I agree the chance of a conflict would be very low. And global assembly functions tend to be rather uniquely named.

Quoted 3 lines

Maybe we could collect all global symbols in asm codes before lld, and then we add an option in lld to feed those global symbols (with a file?), then we can be sure there won't be any conflict?

That wouldn't be worth the effort in my opinion.

I think we should just unconditionally enable -always-rename-promoted-locals=false when it's available. While that will implicitly enforce that global asm functions be uniquely named across the tree, I don't see that as a problem. In the rare case of a conflict, we can just rename the function.

Signature · 1 line
Josh
maintainer
#4 lore raw
Quoted 37 lines · Josh Poimboeuf
Quoted 16 lines · Yonghong Song
Quoted 2 lines

Thanks! Would there be any downsides to enabling this feature unconditionally in the kernel when the compiler supports it?

The only downside is for the following case:

C file: static function foo()
Asm file: global function foo()

The thin-lto will collect all C files and with the above llvm patch, the static function foo() may be promoted to global function foo() if there is no other pre-existing global function foo() in all C files.

In such cases, there will be a conflict since
there are two global function foo() (one from C file, another from Asm file). In such cases, the build will fail.

Quoted 7 lines

How do you think we could hit such issues in linux kernel? Maybe should have default no for the new config?

I think the chance should be very low. The following is a grab for x86 for global symbols in asm code:

[~/work/others/linux/arch/x86 (master)]$ egrep -r globl

There are actually quite a bit more than that, see SYM_CODE_START:

  $ git grep 'SYM_CODE_START(' |wc -l
  169

But still, I agree the chance of a conflict would be very low. And global assembly functions tend to be rather uniquely named.

Quoted 3 lines

Maybe we could collect all global symbols in asm codes before lld, and then we add an option in lld to feed those global symbols (with a file?), then we can be sure there won't be any conflict?

Quoted 7 lines

That wouldn't be worth the effort in my opinion.

I think we should just unconditionally enable -always-rename-promoted-locals=false when it's available. While that will implicitly enforce that global asm functions be uniquely named across the tree, I don't see that as a problem. In the rare case of a conflict, we can just rename the function.

BTW, reading this again I realize that this would affect not only global asm *functions*, but also global asm *data*. But asm global data is quite rare and also tends to be uniquely named, so my conclusion is the same.

Signature · 1 line
Josh
#5 lore raw
Quoted 35 lines · Josh Poimboeuf
Quoted 34 lines · Josh Poimboeuf
Quoted 15 lines · Yonghong Song
Quoted 2 lines

Thanks! Would there be any downsides to enabling this feature unconditionally in the kernel when the compiler supports it?

The only downside is for the following case:

C file: static function foo()
Asm file: global function foo()

The thin-lto will collect all C files and with the above llvm patch, the static function foo() may be promoted to global function foo() if there is no other pre-existing global function foo() in all C files.

In such cases, there will be a conflict since
there are two global function foo() (one from C file, another from Asm file). In such cases, the build will fail.

Quoted 7 lines

How do you think we could hit such issues in linux kernel? Maybe should have default no for the new config?

I think the chance should be very low. The following is a grab for x86 for global symbols in asm code:

[~/work/others/linux/arch/x86 (master)]$ egrep -r globl

There are actually quite a bit more than that, see SYM_CODE_START:

   $ git grep 'SYM_CODE_START(' |wc -l
   169

But still, I agree the chance of a conflict would be very low. And global assembly functions tend to be rather uniquely named.

Quoted 3 lines

Maybe we could collect all global symbols in asm codes before lld, and then we add an option in lld to feed those global symbols (with a file?), then we can be sure there won't be any conflict?

Quoted 7 lines
Quoted 7 lines

That wouldn't be worth the effort in my opinion.

I think we should just unconditionally enable -always-rename-promoted-locals=false when it's available. While that will implicitly enforce that global asm functions be uniquely named across the tree, I don't see that as a problem. In the rare case of a conflict, we can just rename the function.

So you mean we do not need a new config. Should just check clang and clang versionn like below?

Makefile +5 -0
index e944c6e71e81..9bd3990024c5 100644
@@ -1034,6 +1034,11 @@ endif
 ifdef CONFIG_LTO_CLANG
 ifdef CONFIG_LTO_CLANG_THIN
 CC_FLAGS_LTO   := -flto=thin -fsplit-lto-unit
+ifdef CONFIG_CC_IS_CLANG
+ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 230000; echo $$?),0)
+KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false
+endif
+endif
 else
 CC_FLAGS_LTO   := -flto
 endif

I think this should work. In rare case, renaming the function should solve the problem.

Quoted 5 lines

BTW, reading this again I realize that this would affect not only global asm *functions*, but also global asm *data*. But asm global data is quite rare and also tends to be uniquely named, so my conclusion is the same.

With thin-lto, the cross-file data inlining is a side effect of cross-file function inlining. So the number of cross-file data inlining is small. Compared to 1212 number of functions with suffix .llvm.<hash>, the number of data with suffix only 9:

ffffffff84e8ad09 d __func__.net_ratelimit.llvm.12310750674033017485 ffffffff84f1d569 d .str.llvm.11541892489833206993 ffffffff87f42d40 b __cfs_bandwidth_used.llvm.9245315756205243639 ffffffff87f457c0 b __sched_clock_stable.llvm.13183765431478218167 ffffffff881968d0 b lock_chains_in_use.llvm.17116087415131265585 ffffffff88bba180 b timekeeper_data.llvm.4127123038948722377 ffffffff88bd0eb0 b slab_debugfs_root.llvm.537776758349482510 ffffffff88c0b300 b __acpi_os_prepare_sleep.llvm.3969442148434754585 ffffffff88c63b00 b handshake_rhashtbl.llvm.12810142853876704317

With newly added KBUILD_LDFLAGS flags, none of them have suffixes. Again, the conflict for data variable should be rare too.

I will repost the patch with Makefile change once llvm23 release is cut.

Thanks!

#6 lore raw

Makefile endif · line 1038

Quoted 13 lines · Yonghong Song

So you mean we do not need a new config. Should just check clang and clang versionn like below?

Makefile +2 -0
index e944c6e71e81..9bd3990024c5 100644
@@ -1034,6 +1034,11 @@ endif
 ifdef CONFIG_LTO_CLANG
 ifdef CONFIG_LTO_CLANG_THIN
 CC_FLAGS_LTO   := -flto=thin -fsplit-lto-unit
+ifdef CONFIG_CC_IS_CLANG
+ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 230000; echo $$?),0)

This is the same as

  ifeq ($(call clang-min-version,230000),y)

That being said, could we not just do a dynamic check so that it always works when these flags are supported?

Makefile endif · line 1039

Quoted 1 line
Makefile +1 -0
+KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false
  KBUILD_LDFLAGS += $(call ld-option,--lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false)
Quoted 8 lines
Makefile +2 -0
+endif
+endif
 else
 CC_FLAGS_LTO   := -flto
 endif

I think this should work. In rare case, renaming the function should solve the problem.

I agree with Josh that having this be the default behavior with the workarounds that we have available to us to avoid problems from it is worth it. Thanks for chasing this after the discussions at LPC.

Cheers,
Nathan

#7 lore raw
Quoted 16 lines · Nathan Chancellor

Makefile endif · line 1038

Quoted 13 lines · Yonghong Song

So you mean we do not need a new config. Should just check clang and clang versionn like below?

Makefile +2 -0
index e944c6e71e81..9bd3990024c5 100644
@@ -1034,6 +1034,11 @@ endif
  ifdef CONFIG_LTO_CLANG
  ifdef CONFIG_LTO_CLANG_THIN
  CC_FLAGS_LTO   := -flto=thin -fsplit-lto-unit
+ifdef CONFIG_CC_IS_CLANG
+ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 230000; echo $$?),0)

This is the same as

Quoted 4 lines
   ifeq ($(call clang-min-version,230000),y)

That being said, could we not just do a dynamic check so that it always works when these flags are supported?

Thanks for the suggestion! What you had below with 'call ld-option' is perfect to satisfy this requirement: <= llvm22, two flags will be ignored, and

Quoted 1 line

= llvm23, two flags will be used.

v2: https://lore.kernel.org/linux-kbuild/[email protected]/

Quoted 18 lines

Makefile endif · line 1039

Quoted 2 lines
Makefile +1 -0
+KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false

KBUILD_LDFLAGS += $(call ld-option,--lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false)

Quoted 8 lines
Makefile +2 -0
+endif
+endif
  else
  CC_FLAGS_LTO   := -flto
  endif

I think this should work. In rare case, renaming the function should solve the problem.

I agree with Josh that having this be the default behavior with the workarounds that we have available to us to avoid problems from it is worth it. Thanks for chasing this after the discussions at LPC.

Cheers,
Nathan