diff mbox series

[2/2] Makefile: Only specify '--prefix=' when building with clang + GNU as

Message ID 20210302210646.3044738-2-nathan@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/2] Makefile: Remove '--gcc-toolchain' flag | expand

Commit Message

Nathan Chancellor March 2, 2021, 9:06 p.m. UTC 
When building with LLVM_IAS=1, there is no point to specifying
'--prefix=' because that flag is only used to find the cross assembler,
which is clang itself when building with LLVM_IAS=1. All of the other
tools are invoked directly from PATH or a full path specified via the
command line, which does not depend on the value of '--prefix='.

Sharing commands to reproduce issues becomes a little bit easier without
a '--prefix=' value because that '--prefix=' value is specific to a
user's machine due to it being an absolute path.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Fangrui Song March 2, 2021, 10:02 p.m. UTC | #1 
On 2021-03-02, Nathan Chancellor wrote:
>When building with LLVM_IAS=1, there is no point to specifying
>'--prefix=' because that flag is only used to find the cross assembler,
>which is clang itself when building with LLVM_IAS=1. All of the other
>tools are invoked directly from PATH or a full path specified via the
>command line, which does not depend on the value of '--prefix='.
>
>Sharing commands to reproduce issues becomes a little bit easier without
>a '--prefix=' value because that '--prefix=' value is specific to a
>user's machine due to it being an absolute path.
>
>Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Fangrui Song <maskray@google.com>

clang can spawn GNU as (if -f?no-integrated-as is specified) and GNU
objcopy (-f?no-integrated-as and -gsplit-dwarf and -g[123]).

With LLVM_IAS=1, these cases are ruled out.
Nick Desaulniers March 2, 2021, 10:09 p.m. UTC | #2 
On Tue, Mar 2, 2021 at 2:02 PM Fangrui Song <maskray@google.com> wrote:
>
> On 2021-03-02, Nathan Chancellor wrote:
> >When building with LLVM_IAS=1, there is no point to specifying
> >'--prefix=' because that flag is only used to find the cross assembler,
> >which is clang itself when building with LLVM_IAS=1. All of the other
> >tools are invoked directly from PATH or a full path specified via the
> >command line, which does not depend on the value of '--prefix='.
> >
> >Sharing commands to reproduce issues becomes a little bit easier without
> >a '--prefix=' value because that '--prefix=' value is specific to a
> >user's machine due to it being an absolute path.
> >
> >Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>
> Reviewed-by: Fangrui Song <maskray@google.com>
>
> clang can spawn GNU as (if -f?no-integrated-as is specified) and GNU
> objcopy (-f?no-integrated-as and -gsplit-dwarf and -g[123]).

But -g get's set via CONFIG_DEBUG_INFO and -gsplit-dwarf by
DEBUG_INFO_SPLIT.  So if we say:
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang LLVM_IAS=1

So cross compile, use clang, use the integrated assembler (ie. with
this change, don't set --prefix), with either of the two above
configs, which objcopy get's exec'd?

>
> With LLVM_IAS=1, these cases are ruled out.
Nick Desaulniers March 2, 2021, 10:40 p.m. UTC | #3 
On Tue, Mar 2, 2021 at 2:09 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Tue, Mar 2, 2021 at 2:02 PM Fangrui Song <maskray@google.com> wrote:
> >
> > On 2021-03-02, Nathan Chancellor wrote:
> > >When building with LLVM_IAS=1, there is no point to specifying
> > >'--prefix=' because that flag is only used to find the cross assembler,
> > >which is clang itself when building with LLVM_IAS=1. All of the other
> > >tools are invoked directly from PATH or a full path specified via the
> > >command line, which does not depend on the value of '--prefix='.
> > >
> > >Sharing commands to reproduce issues becomes a little bit easier without
> > >a '--prefix=' value because that '--prefix=' value is specific to a
> > >user's machine due to it being an absolute path.
> > >
> > >Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> >
> > Reviewed-by: Fangrui Song <maskray@google.com>
> >
> > clang can spawn GNU as (if -f?no-integrated-as is specified) and GNU
> > objcopy (-f?no-integrated-as and -gsplit-dwarf and -g[123]).
>
> But -g get's set via CONFIG_DEBUG_INFO and -gsplit-dwarf by
> DEBUG_INFO_SPLIT.  So if we say:
> $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang LLVM_IAS=1
>
> So cross compile, use clang, use the integrated assembler (ie. with
> this change, don't set --prefix), with either of the two above
> configs, which objcopy get's exec'd?

Ok, I spoke to Fangrui more offline, and probably misread his
response. From our chat:
```
Fangrui:
objcopy is only used for GNU as assembled object files
With integrated assembler, the object file streamer creates .o and
.dwo simultaneously
With GNU as, two objcopy commands are needed to extract .debug*.dwo to
.dwo files &&& another command to remove .debug*.dwo sections
```

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

I ran this series through a mix of LLVM=1 vs CC=clang, LLVM_IAS=1 vs
unset, CROSS_COMPILE vs not, without issue.

>
> >
> > With LLVM_IAS=1, these cases are ruled out.
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
Masahiro Yamada March 9, 2021, 7:55 p.m. UTC | #4 
On Wed, Mar 3, 2021 at 6:07 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> When building with LLVM_IAS=1, there is no point to specifying
> '--prefix=' because that flag is only used to find the cross assembler,
> which is clang itself when building with LLVM_IAS=1. All of the other
> tools are invoked directly from PATH or a full path specified via the
> command line, which does not depend on the value of '--prefix='.
>
> Sharing commands to reproduce issues becomes a little bit easier without
> a '--prefix=' value because that '--prefix=' value is specific to a
> user's machine due to it being an absolute path.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>


I was tricked by a couple of Reviewed-by/Tested-by tags.

With this patch applied, the code looks as follows:


ifneq ($(CROSS_COMPILE),)
CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
ifneq ($(LLVM_IAS),1)
GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
CLANG_FLAGS += -no-integrated-as
endif
endif


For the native build (empty CROSS_COMPILE),
you cannot add -no-integrated-as.


I dropped this from my tree.



Is the correct code as follows?


ifneq ($(LLVM_IAS),1)
CLANG_FLAGS += -no-integrated-as
ifneq ($(CROSS_COMPILE),)
CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
endif
endif






> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c20f0ad8be73..0413b8c594cd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -566,12 +566,12 @@ CC_VERSION_TEXT = $(shell $(CC) --version 2>/dev/null | head -n 1 | sed 's/\#//g
>  ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
>  ifneq ($(CROSS_COMPILE),)
>  CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
> +ifneq ($(LLVM_IAS),1)
>  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> -endif
> -ifneq ($(LLVM_IAS),1)
>  CLANG_FLAGS    += -no-integrated-as
>  endif
> +endif
>  CLANG_FLAGS    += -Werror=unknown-warning-option
>  KBUILD_CFLAGS  += $(CLANG_FLAGS)
>  KBUILD_AFLAGS  += $(CLANG_FLAGS)
> --
> 2.31.0.rc0.75.gec125d1bc1
>
Masahiro Yamada March 9, 2021, 7:58 p.m. UTC | #5 
On Wed, Mar 10, 2021 at 4:55 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Mar 3, 2021 at 6:07 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > When building with LLVM_IAS=1, there is no point to specifying
> > '--prefix=' because that flag is only used to find the cross assembler,
> > which is clang itself when building with LLVM_IAS=1. All of the other
> > tools are invoked directly from PATH or a full path specified via the
> > command line, which does not depend on the value of '--prefix='.
> >
> > Sharing commands to reproduce issues becomes a little bit easier without
> > a '--prefix=' value because that '--prefix=' value is specific to a
> > user's machine due to it being an absolute path.
> >
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>
>
> I was tricked by a couple of Reviewed-by/Tested-by tags.
>
> With this patch applied, the code looks as follows:
>
>
> ifneq ($(CROSS_COMPILE),)
> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> ifneq ($(LLVM_IAS),1)
> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> CLANG_FLAGS += -no-integrated-as
> endif
> endif
>
>
> For the native build (empty CROSS_COMPILE),
> you cannot add -no-integrated-as.
>
>
> I dropped this from my tree.
>
>
>
> Is the correct code as follows?
>
>
> ifneq ($(LLVM_IAS),1)
> CLANG_FLAGS += -no-integrated-as
> ifneq ($(CROSS_COMPILE),)
> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> endif
> endif
>
>
>
>


If you send v2, can you include comments from Fangrui Song?



    Fangrui Song:
      clang can spawn GNU as (if -f?no-integrated-as is specified) and GNU
      objcopy (-f?no-integrated-as and -gsplit-dwarf and -g[123]).
      objcopy is only used for GNU as assembled object files.
      With integrated assembler, the object file streamer creates .o and
      .dwo simultaneously.
      With GNU as, two objcopy commands are needed to extract .debug*.dwo to
      .dwo files && another command to remove .debug*.dwo sections.



I did not know the objtool part, and I think it is worth recording.
kernel test robot Sept. 29, 2021, 5:26 p.m. UTC | #6 
Hi Nathan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 7a7fd0de4a9804299793e564a555a49c1fc924cb]

url:    https://github.com/0day-ci/linux/commits/Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
base:   7a7fd0de4a9804299793e564a555a49c1fc924cb
config: x86_64-randconfig-a014-20210929 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project dc6e8dfdfe7efecfda318d43a06fae18b40eb498)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/74a168f9e88d9edf5aaa8209d2c39e676de2857a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
        git checkout 74a168f9e88d9edf5aaa8209d2c39e676de2857a
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/entry/ arch/x86/kernel/ arch/x86/lib/ kernel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> arch/x86/entry/entry_64.S:47:1: warning: DWARF2 only supports one section per compilation unit
   .section .entry.text, "ax"
   ^
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   <instantiation>:1:1: note: while in macro instantiation
   ALTERNATIVE "jmp .Lend_2", "", ( 7*32+11)
   ^
   arch/x86/entry/entry_64.S:93:2: note: while in macro instantiation
    SWITCH_TO_KERNEL_CR3 scratch_reg=%rsp
    ^
   arch/x86/entry/entry_64.S:741:2: warning: DWARF2 only supports one section per compilation unit
    .section .fixup, "ax"
    ^
--
>> arch/x86/entry/entry_64_compat.S:20:2: warning: DWARF2 only supports one section per compilation unit
    .section .entry.text, "ax"
    ^
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   <instantiation>:1:1: note: while in macro instantiation
   ALTERNATIVE "jmp .Lend_2", "", ( 7*32+11)
   ^
   arch/x86/entry/entry_64_compat.S:55:2: note: while in macro instantiation
    SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
    ^
--
>> arch/x86/kernel/head_64.S:42:2: warning: DWARF2 only supports one section per compilation unit
    .section ".head.text","ax"
    ^
   arch/x86/kernel/head_64.S:352:2: warning: DWARF2 only supports one section per compilation unit
    .section ".init.text","ax"
    ^
--
>> arch/x86/lib/copy_mc_64.S:138:2: warning: DWARF2 only supports one section per compilation unit
    .section .fixup, "ax"
    ^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   arch/x86/lib/copy_page_64.S:17:2: note: while in macro instantiation
    ALTERNATIVE "jmp copy_page_regs", "", ( 3*32+16)
    ^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   arch/x86/lib/copy_user_64.S:58:2: note: while in macro instantiation
    ALTERNATIVE "", ".byte 0x0f,0x01,0xcb", ( 9*32+20)
    ^
   <instantiation>:15:2: warning: DWARF2 only supports one section per compilation unit
    .section .fixup,"ax"
    ^
   arch/x86/lib/copy_user_64.S:61:2: note: while in macro instantiation
    ALIGN_DESTINATION
    ^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   arch/x86/lib/getuser.S:56:2: note: while in macro instantiation
    ALTERNATIVE "", ".byte 0x0f,0x01,0xcb", ( 9*32+20)
    ^
--
>> arch/x86/lib/memcpy_64.S:10:1: warning: DWARF2 only supports one section per compilation unit
   .pushsection .noinstr.text, "ax"
   ^
   <instantiation>:13:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   arch/x86/lib/memcpy_64.S:32:2: note: while in macro instantiation
    ALTERNATIVE_2 "jmp memcpy_orig", "", ( 3*32+16), "jmp memcpy_erms", ( 9*32+ 9)
    ^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   arch/x86/lib/memmove_64.S:42:2: note: while in macro instantiation
    ALTERNATIVE "cmp $0x20, %rdx; jb 1f", "", (18*32+ 4)
    ^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
    .pushsection .altinstr_replacement,"ax"
    ^
   arch/x86/lib/putuser.S:51:2: note: while in macro instantiation
    ALTERNATIVE "", ".byte 0x0f,0x01,0xcb", ( 9*32+20)
    ^


vim +47 arch/x86/entry/entry_64.S

6fd166aae78c0a arch/x86/entry/entry_64.S  Peter Zijlstra 2017-12-04  45  
^1da177e4c3f41 arch/x86_64/kernel/entry.S Linus Torvalds 2005-04-16  46  .code64
ea7145477a461e arch/x86/kernel/entry_64.S Jiri Olsa      2011-03-07 @47  .section .entry.text, "ax"
ea7145477a461e arch/x86/kernel/entry_64.S Jiri Olsa      2011-03-07  48  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Sept. 29, 2021, 7:32 p.m. UTC | #7 
Hi Nathan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 7a7fd0de4a9804299793e564a555a49c1fc924cb]

url:    https://github.com/0day-ci/linux/commits/Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
base:   7a7fd0de4a9804299793e564a555a49c1fc924cb
config: x86_64-randconfig-a011-20210929 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project dc6e8dfdfe7efecfda318d43a06fae18b40eb498)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/74a168f9e88d9edf5aaa8209d2c39e676de2857a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
        git checkout 74a168f9e88d9edf5aaa8209d2c39e676de2857a
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> arch/x86/platform/pvh/head.S:22:2: warning: DWARF2 only supports one section per compilation unit
    .section ".head.text","ax"
    ^


vim +22 arch/x86/platform/pvh/head.S

7243b93345f7f8d arch/x86/xen/xen-pvh.S Boris Ostrovsky 2017-02-05  21  
7243b93345f7f8d arch/x86/xen/xen-pvh.S Boris Ostrovsky 2017-02-05 @22  	__HEAD
7243b93345f7f8d arch/x86/xen/xen-pvh.S Boris Ostrovsky 2017-02-05  23  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index c20f0ad8be73..0413b8c594cd 100644
--- a/Makefile
+++ b/Makefile
@@ -566,12 +566,12 @@  CC_VERSION_TEXT = $(shell $(CC) --version 2>/dev/null | head -n 1 | sed 's/\#//g
 ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
 ifneq ($(CROSS_COMPILE),)
 CLANG_FLAGS	+= --target=$(notdir $(CROSS_COMPILE:%-=%))
+ifneq ($(LLVM_IAS),1)
 GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
 CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
-endif
-ifneq ($(LLVM_IAS),1)
 CLANG_FLAGS	+= -no-integrated-as
 endif
+endif
 CLANG_FLAGS	+= -Werror=unknown-warning-option
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)