Re: [PATCH 27/27] kbuild: do not create *.prelink.o for Clang LTO or IBT

[Nicolas Schier <nicolas@fjasle.eu>]

On Thu 28 Apr 2022 13:38:39 GMT Masahiro Yamada wrote:
> 
> On Thu, Apr 28, 2022 at 12:31 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
> >
> > On Mon 25 Apr 2022 04:08:11 GMT Masahiro Yamada wrote:
> > > When CONFIG_LTO_CLANG=y, additional intermediate *.prelink.o is
> > > created
> > > for each module. Also, objtool is postponed until LLVM bitcode is
> > > converted to ELF.
> > >
> > > CONFIG_X86_KERNEL_IBT works in a similar way to postpone objtool until
> > > objects are merged together.
> > >
> > > This commit stops generating *.prelink.o, so the build flow will look
> > > the same with/without LTO.
> > >
> > > The following figures show how the LTO build currently works, and
> > > how this commit is changing it.
> > >
> > > Current build flow
> > > ==================
> > >
> > >  [1] single-object module
> > >
> > >                                     [+objtool]
> > >            $(CC)                      $(LD)                $(LD)
> > >     foo.c --------------------> foo.o -----> foo.prelink.o -----> foo.ko
> > >                            (LLVM bitcode)        (ELF)       |
> > >                                                              |
> > >                                                  foo.mod.o --/
> > >
> > >  [2] multi-object module
> > >                                     [+objtool]
> > >            $(CC)         $(AR)        $(LD)                $(LD)
> > >     foo1.c -----> foo1.o -----> foo.o -----> foo.prelink.o -----> foo.ko
> > >                            |  (archive)          (ELF)       |
> > >     foo2.c -----> foo2.o --/                                 |
> > >                 (LLVM bitcode)                   foo.mod.o --/
> > >
> > >   One confusion is foo.o in multi-object module is an archive despite of
> > >   its suffix.
> > >
> > > New build flow
> > > ==============
> > >
> > >  [1] single-object module
> > >
> > >   Since there is only one object, we do not need to have the LLVM
> > >   bitcode stage. Use $(CC)+$(LD) to generate an ELF object in one
> > >   build rule. Of course, only $(CC) is used when LTO is disabled.
> > >
> > >            $(CC)+$(LD)[+objtool]           $(LD)
> > >     foo.c ------------------------> foo.o -------> foo.ko
> > >                                     (ELF)    |
> > >                                              |
> > >                                  foo.mod.o --/
> > >
> > >  [2] multi-object module
> > >
> > >   Previously, $(AR) was used to combine LLVM bitcode into an archive,
> > >   but there was not a technical reason to do so.
> > >   This commit just uses $(LD) to combine and convert them into a single
> > >   ELF object.
> > >
> > >                           [+objtool]
> > >             $(CC)           $(LD)          $(LD)
> > >     foo1.c -------> foo1.o -------> foo.o -------> foo.ko
> > >                               |     (ELF)    |
> > >     foo2.c -------> foo2.o ---/              |
> > >                 (LLVM bitcode)   foo.mod.o --/
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > >  scripts/Kbuild.include    |  4 +++
> > >  scripts/Makefile.build    | 58 ++++++++++++---------------------------
> > >  scripts/Makefile.lib      |  7 -----
> > >  scripts/Makefile.modfinal |  5 ++--
> > >  scripts/Makefile.modpost  |  9 ++----
> > >  scripts/mod/modpost.c     |  7 -----
> > >  6 files changed, 25 insertions(+), 65 deletions(-)
> > >
> > > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> > > index 3514c2149e9d..455a0a6ce12d 100644
> > > --- a/scripts/Kbuild.include
> > > +++ b/scripts/Kbuild.include
> > > @@ -15,6 +15,10 @@ pound := \#
> > >  # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
> > >  dot-target = $(dir $@).$(notdir $@)
> > >
> > > +###
> > > +# Name of target with a '.tmp_' as filename prefix. foo/bar.o => foo/.tmp_bar.o
> > > +tmp-target = $(dir $@).tmp_$(notdir $@)
> >
> > This matches the dot-target definition above, otherwise $(@D).tmp_$(@F)
> > would be an alternative.
> 
> Yes, I intentionally made dot-target and tmp-target look similar.
> 
> The difference is $(dir ...) leaves the trailing slash, but $(@D) does not.
> 
> The alternative would be
> $(@D)/.tmp_$(@F)

ah, yes, thanks.

> >
> > > +
> > >  ###
> > >  # The temporary file to save gcc -MMD generated dependencies must not
> > >  # contain a comma
> > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > index 7f199b0a5170..fe4d3a908dd0 100644
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -88,10 +88,6 @@ endif
> > >  targets-for-modules := $(foreach x, o mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
> > >                               $(patsubst %.o, %.$x, $(filter %.o, $(obj-m))))
> > >
> > > -ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
> > > -targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
> > > -endif
> > > -
> > >  ifdef need-modorder
> > >  targets-for-modules += $(obj)/modules.order
> > >  endif
> > > @@ -153,8 +149,16 @@ $(obj)/%.ll: $(src)/%.c FORCE
> > >  # The C file is compiled and updated dependency information is generated.
> > >  # (See cmd_cc_o_c + relevant part of rule_cc_o_c)
> > >
> > > +is-single-obj-m = $(if $(part-of-module),$(if $(filter $@, $(obj-m)),y))
> >
> > Perhaps using $(and ..,..,y) instead if $(if ..,$(if ..,y))?
> 
> 
> Good idea!
> I did not notice this.  I will do it in v2.
> 
> 
> 
> > >
> > > -endif
> > > +delay-objtool := $(or $(CONFIG_LTO_CLANG),$(CONFIG_X86_KERNEL_IBT))
> > > +
> > > +$(obj)/%.o: objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
> >
> > same here?  $(and) versus $(if ..,$(if ..,y))
> 
> I am not sure about this case.
> The second if-func is  $(if  cond, A, B) form.

Oh, I didn't see it.  Then it might be the best to keep it the way it 
is.