linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
@ 2021-08-02 23:43 Nick Desaulniers
  2021-08-03  1:32 ` Matthew Wilcox
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Nick Desaulniers @ 2021-08-02 23:43 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor
  Cc: Khem Raj, Nick Desaulniers, Michal Marek, Jonathan Corbet,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, clang-built-linux,
	linux-kbuild, linux-doc, linux-kernel, linux-riscv

LLVM_IAS=1 controls enabling clang's integrated assembler via
-integrated-as. This was an explicit opt in until we could enable
assembler support in Clang for more architecures. Now we have support
and CI coverage of LLVM_IAS=1 for all architecures except a few more
bugs affecting s390 and powerpc.

This commit flips the default from opt in via LLVM_IAS=1 to opt out via
LLVM_IAS=0.  CI systems or developers that were previously doing builds
with CC=clang or LLVM=1 without explicitly setting LLVM_IAS must now
explicitly opt out via LLVM_IAS=0, otherwise they will be implicitly
opted-in.

This finally shortens the command line invocation when cross compiling
with LLVM to simply:

$ make ARCH=arm64 LLVM=1

Link: https://github.com/ClangBuiltLinux/linux/issues/1434
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Note: base is:
https://lore.kernel.org/lkml/20210802183910.1802120-1-ndesaulniers@google.com/

 Documentation/kbuild/llvm.rst | 14 ++++++++------
 Makefile                      |  2 +-
 arch/riscv/Makefile           |  2 +-
 scripts/Makefile.clang        |  6 +++---
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
index f8a360958f4c..16712fab4d3a 100644
--- a/Documentation/kbuild/llvm.rst
+++ b/Documentation/kbuild/llvm.rst
@@ -60,17 +60,14 @@ They can be enabled individually. The full list of the parameters: ::
 	  OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
 	  HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
 
-Currently, the integrated assembler is disabled by default. You can pass
-``LLVM_IAS=1`` to enable it.
+Currently, the integrated assembler is enabled by default. You can pass
+``LLVM_IAS=0`` to disable it.
 
 Omitting CROSS_COMPILE
 ----------------------
 
 As explained above, ``CROSS_COMPILE`` is used to set ``--target=<triple>``.
 
-Unless ``LLVM_IAS=1`` is specified, ``CROSS_COMPILE`` is also used to derive
-``--prefix=<path>`` to search for the GNU assembler and linker.
-
 If ``CROSS_COMPILE`` is not specified, the ``--target=<triple>`` is inferred
 from ``ARCH``.
 
@@ -78,7 +75,12 @@ That means if you use only LLVM tools, ``CROSS_COMPILE`` becomes unnecessary.
 
 For example, to cross-compile the arm64 kernel::
 
-	make ARCH=arm64 LLVM=1 LLVM_IAS=1
+	make ARCH=arm64 LLVM=1
+
+If ``LLVM_IAS=0`` is specified, ``CROSS_COMPILE`` is also used to derive
+``--prefix=<path>`` to search for the GNU assembler and linker. ::
+
+	make ARCH=arm64 LLVM=1 LLVM_IAS=0 CROSS_COMPILE=aarch64-linux-gnu-
 
 Supported Architectures
 -----------------------
diff --git a/Makefile b/Makefile
index 444558e62cbc..b24b48c9ebb7 100644
--- a/Makefile
+++ b/Makefile
@@ -845,7 +845,7 @@ else
 DEBUG_CFLAGS	+= -g
 endif
 
-ifneq ($(LLVM_IAS),1)
+ifeq ($(LLVM_IAS),0)
 KBUILD_AFLAGS	+= -Wa,-gdwarf-2
 endif
 
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index bc74afdbf31e..807f7c94bc6f 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -41,7 +41,7 @@ endif
 ifeq ($(CONFIG_LD_IS_LLD),y)
 	KBUILD_CFLAGS += -mno-relax
 	KBUILD_AFLAGS += -mno-relax
-ifneq ($(LLVM_IAS),1)
+ifeq ($(LLVM_IAS),0)
 	KBUILD_CFLAGS += -Wa,-mno-relax
 	KBUILD_AFLAGS += -Wa,-mno-relax
 endif
diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
index 1f4e3eb70f88..3ae63bd35582 100644
--- a/scripts/Makefile.clang
+++ b/scripts/Makefile.clang
@@ -22,12 +22,12 @@ else
 CLANG_FLAGS	+= --target=$(notdir $(CROSS_COMPILE:%-=%))
 endif # CROSS_COMPILE
 
-ifeq ($(LLVM_IAS),1)
-CLANG_FLAGS	+= -integrated-as
-else
+ifeq ($(LLVM_IAS),0)
 CLANG_FLAGS	+= -no-integrated-as
 GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
 CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
+else
+CLANG_FLAGS	+= -integrated-as
 endif
 CLANG_FLAGS	+= -Werror=unknown-warning-option
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)

base-commit: d7a86429dbc691bf540688fcc8542cc20246a85b
prerequisite-patch-id: 0d3072ecb5fd06ff6fd6ea81fe601f6c54c23910
prerequisite-patch-id: 2654829756eb8a094a0ffad1679caa75a4d86619
prerequisite-patch-id: a51e7885ca2376d008bbf146a5589da247806f7b
-- 
2.32.0.554.ge1b32706d8-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
  2021-08-02 23:43 [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1 Nick Desaulniers
@ 2021-08-03  1:32 ` Matthew Wilcox
  2021-08-05 13:59   ` Masahiro Yamada
  2021-08-03  1:42 ` Khem Raj
  2021-08-05 15:15 ` Masahiro Yamada
  2 siblings, 1 reply; 8+ messages in thread
From: Matthew Wilcox @ 2021-08-03  1:32 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Masahiro Yamada, Nathan Chancellor, Khem Raj, Michal Marek,
	Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	clang-built-linux, linux-kbuild, linux-doc, linux-kernel,
	linux-riscv

On Mon, Aug 02, 2021 at 04:43:03PM -0700, Nick Desaulniers wrote:
> +++ b/Documentation/kbuild/llvm.rst
> @@ -60,17 +60,14 @@ They can be enabled individually. The full list of the parameters: ::
>  	  OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
>  	  HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
>  
> -Currently, the integrated assembler is disabled by default. You can pass
> -``LLVM_IAS=1`` to enable it.
> +Currently, the integrated assembler is enabled by default. You can pass
> +``LLVM_IAS=0`` to disable it.

I'd drop the "Currently,".  This is presumably going to be the default
going forward unless there's some horrible unforeseen problem.  The
"Currently," implies that we're planning on changing it.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
  2021-08-02 23:43 [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1 Nick Desaulniers
  2021-08-03  1:32 ` Matthew Wilcox
@ 2021-08-03  1:42 ` Khem Raj
  2021-08-05 15:15 ` Masahiro Yamada
  2 siblings, 0 replies; 8+ messages in thread
From: Khem Raj @ 2021-08-03  1:42 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Masahiro Yamada, Nathan Chancellor, Michal Marek,
	Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	clang-built-linux, linux-kbuild, linux-doc, linux-kernel,
	linux-riscv

On Mon, Aug 2, 2021 at 4:43 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> LLVM_IAS=1 controls enabling clang's integrated assembler via
> -integrated-as. This was an explicit opt in until we could enable
> assembler support in Clang for more architecures. Now we have support
> and CI coverage of LLVM_IAS=1 for all architecures except a few more
> bugs affecting s390 and powerpc.
>
> This commit flips the default from opt in via LLVM_IAS=1 to opt out via
> LLVM_IAS=0.  CI systems or developers that were previously doing builds
> with CC=clang or LLVM=1 without explicitly setting LLVM_IAS must now
> explicitly opt out via LLVM_IAS=0, otherwise they will be implicitly
> opted-in.
>
> This finally shortens the command line invocation when cross compiling
> with LLVM to simply:
>
> $ make ARCH=arm64 LLVM=1
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1434
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Note: base is:
> https://lore.kernel.org/lkml/20210802183910.1802120-1-ndesaulniers@google.com/
>
>  Documentation/kbuild/llvm.rst | 14 ++++++++------
>  Makefile                      |  2 +-
>  arch/riscv/Makefile           |  2 +-
>  scripts/Makefile.clang        |  6 +++---
>  4 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> index f8a360958f4c..16712fab4d3a 100644
> --- a/Documentation/kbuild/llvm.rst
> +++ b/Documentation/kbuild/llvm.rst
> @@ -60,17 +60,14 @@ They can be enabled individually. The full list of the parameters: ::
>           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
>           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
>
> -Currently, the integrated assembler is disabled by default. You can pass
> -``LLVM_IAS=1`` to enable it.
> +Currently, the integrated assembler is enabled by default. You can pass
> +``LLVM_IAS=0`` to disable it.
>
>  Omitting CROSS_COMPILE
>  ----------------------
>
>  As explained above, ``CROSS_COMPILE`` is used to set ``--target=<triple>``.
>
> -Unless ``LLVM_IAS=1`` is specified, ``CROSS_COMPILE`` is also used to derive
> -``--prefix=<path>`` to search for the GNU assembler and linker.
> -
>  If ``CROSS_COMPILE`` is not specified, the ``--target=<triple>`` is inferred
>  from ``ARCH``.
>
> @@ -78,7 +75,12 @@ That means if you use only LLVM tools, ``CROSS_COMPILE`` becomes unnecessary.
>
>  For example, to cross-compile the arm64 kernel::
>
> -       make ARCH=arm64 LLVM=1 LLVM_IAS=1
> +       make ARCH=arm64 LLVM=1
> +
> +If ``LLVM_IAS=0`` is specified, ``CROSS_COMPILE`` is also used to derive
> +``--prefix=<path>`` to search for the GNU assembler and linker. ::
> +

I am not sure if LLVM_IAS should also impacts linker selection

> +       make ARCH=arm64 LLVM=1 LLVM_IAS=0 CROSS_COMPILE=aarch64-linux-gnu-
>
>  Supported Architectures
>  -----------------------
> diff --git a/Makefile b/Makefile
> index 444558e62cbc..b24b48c9ebb7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -845,7 +845,7 @@ else
>  DEBUG_CFLAGS   += -g
>  endif
>
> -ifneq ($(LLVM_IAS),1)
> +ifeq ($(LLVM_IAS),0)
>  KBUILD_AFLAGS  += -Wa,-gdwarf-2
>  endif
>
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index bc74afdbf31e..807f7c94bc6f 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -41,7 +41,7 @@ endif
>  ifeq ($(CONFIG_LD_IS_LLD),y)
>         KBUILD_CFLAGS += -mno-relax
>         KBUILD_AFLAGS += -mno-relax
> -ifneq ($(LLVM_IAS),1)
> +ifeq ($(LLVM_IAS),0)
>         KBUILD_CFLAGS += -Wa,-mno-relax
>         KBUILD_AFLAGS += -Wa,-mno-relax
>  endif
> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 1f4e3eb70f88..3ae63bd35582 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -22,12 +22,12 @@ else
>  CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
>  endif # CROSS_COMPILE
>
> -ifeq ($(LLVM_IAS),1)
> -CLANG_FLAGS    += -integrated-as
> -else
> +ifeq ($(LLVM_IAS),0)
>  CLANG_FLAGS    += -no-integrated-as
>  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> +else
> +CLANG_FLAGS    += -integrated-as
>  endif
>  CLANG_FLAGS    += -Werror=unknown-warning-option
>  KBUILD_CFLAGS  += $(CLANG_FLAGS)
>
> base-commit: d7a86429dbc691bf540688fcc8542cc20246a85b
> prerequisite-patch-id: 0d3072ecb5fd06ff6fd6ea81fe601f6c54c23910
> prerequisite-patch-id: 2654829756eb8a094a0ffad1679caa75a4d86619
> prerequisite-patch-id: a51e7885ca2376d008bbf146a5589da247806f7b
> --
> 2.32.0.554.ge1b32706d8-goog
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
  2021-08-03  1:32 ` Matthew Wilcox
@ 2021-08-05 13:59   ` Masahiro Yamada
  0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-05 13:59 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Nick Desaulniers, Nathan Chancellor, Khem Raj, Michal Marek,
	Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	clang-built-linux, Linux Kbuild mailing list,
	open list:DOCUMENTATION, Linux Kernel Mailing List,
	open list:SIFIVE DRIVERS

On Tue, Aug 3, 2021 at 10:33 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Aug 02, 2021 at 04:43:03PM -0700, Nick Desaulniers wrote:
> > +++ b/Documentation/kbuild/llvm.rst
> > @@ -60,17 +60,14 @@ They can be enabled individually. The full list of the parameters: ::
> >         OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
> >         HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
> >
> > -Currently, the integrated assembler is disabled by default. You can pass
> > -``LLVM_IAS=1`` to enable it.
> > +Currently, the integrated assembler is enabled by default. You can pass
> > +``LLVM_IAS=0`` to disable it.
>
> I'd drop the "Currently,".  This is presumably going to be the default
> going forward unless there's some horrible unforeseen problem.  The
> "Currently," implies that we're planning on changing it.

I agree.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
  2021-08-02 23:43 [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1 Nick Desaulniers
  2021-08-03  1:32 ` Matthew Wilcox
  2021-08-03  1:42 ` Khem Raj
@ 2021-08-05 15:15 ` Masahiro Yamada
  2021-08-05 15:18   ` Khem Raj
  2021-08-05 18:40   ` Nick Desaulniers
  2 siblings, 2 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-05 15:15 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Khem Raj, Michal Marek, Jonathan Corbet,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, clang-built-linux,
	Linux Kbuild mailing list, open list:DOCUMENTATION,
	Linux Kernel Mailing List, open list:SIFIVE DRIVERS

On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> LLVM_IAS=1 controls enabling clang's integrated assembler via
> -integrated-as. This was an explicit opt in until we could enable
> assembler support in Clang for more architecures. Now we have support
> and CI coverage of LLVM_IAS=1 for all architecures except a few more
> bugs affecting s390 and powerpc.
>
> This commit flips the default from opt in via LLVM_IAS=1 to opt out via
> LLVM_IAS=0.  CI systems or developers that were previously doing builds
> with CC=clang or LLVM=1 without explicitly setting LLVM_IAS must now
> explicitly opt out via LLVM_IAS=0, otherwise they will be implicitly
> opted-in.
>
> This finally shortens the command line invocation when cross compiling
> with LLVM to simply:
>
> $ make ARCH=arm64 LLVM=1
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1434
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Note: base is:
> https://lore.kernel.org/lkml/20210802183910.1802120-1-ndesaulniers@google.com/
>
>  Documentation/kbuild/llvm.rst | 14 ++++++++------
>  Makefile                      |  2 +-
>  arch/riscv/Makefile           |  2 +-
>  scripts/Makefile.clang        |  6 +++---
>  4 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> index f8a360958f4c..16712fab4d3a 100644
> --- a/Documentation/kbuild/llvm.rst
> +++ b/Documentation/kbuild/llvm.rst
> @@ -60,17 +60,14 @@ They can be enabled individually. The full list of the parameters: ::
>           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
>           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
>
> -Currently, the integrated assembler is disabled by default. You can pass
> -``LLVM_IAS=1`` to enable it.
> +Currently, the integrated assembler is enabled by default. You can pass
> +``LLVM_IAS=0`` to disable it.
>
>  Omitting CROSS_COMPILE
>  ----------------------
>
>  As explained above, ``CROSS_COMPILE`` is used to set ``--target=<triple>``.
>
> -Unless ``LLVM_IAS=1`` is specified, ``CROSS_COMPILE`` is also used to derive
> -``--prefix=<path>`` to search for the GNU assembler and linker.
> -
>  If ``CROSS_COMPILE`` is not specified, the ``--target=<triple>`` is inferred
>  from ``ARCH``.
>
> @@ -78,7 +75,12 @@ That means if you use only LLVM tools, ``CROSS_COMPILE`` becomes unnecessary.
>
>  For example, to cross-compile the arm64 kernel::
>
> -       make ARCH=arm64 LLVM=1 LLVM_IAS=1
> +       make ARCH=arm64 LLVM=1
> +
> +If ``LLVM_IAS=0`` is specified, ``CROSS_COMPILE`` is also used to derive
> +``--prefix=<path>`` to search for the GNU assembler and linker. ::
> +
> +       make ARCH=arm64 LLVM=1 LLVM_IAS=0 CROSS_COMPILE=aarch64-linux-gnu-
>
>  Supported Architectures
>  -----------------------
> diff --git a/Makefile b/Makefile
> index 444558e62cbc..b24b48c9ebb7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -845,7 +845,7 @@ else
>  DEBUG_CFLAGS   += -g
>  endif
>
> -ifneq ($(LLVM_IAS),1)
> +ifeq ($(LLVM_IAS),0)
>  KBUILD_AFLAGS  += -Wa,-gdwarf-2
>  endif
>
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index bc74afdbf31e..807f7c94bc6f 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -41,7 +41,7 @@ endif
>  ifeq ($(CONFIG_LD_IS_LLD),y)
>         KBUILD_CFLAGS += -mno-relax
>         KBUILD_AFLAGS += -mno-relax
> -ifneq ($(LLVM_IAS),1)
> +ifeq ($(LLVM_IAS),0)
>         KBUILD_CFLAGS += -Wa,-mno-relax
>         KBUILD_AFLAGS += -Wa,-mno-relax
>  endif



Please drop these two hunks.

I will apply my patch instead.
https://lore.kernel.org/patchwork/patch/1472580/






> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 1f4e3eb70f88..3ae63bd35582 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -22,12 +22,12 @@ else
>  CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
>  endif # CROSS_COMPILE
>
> -ifeq ($(LLVM_IAS),1)
> -CLANG_FLAGS    += -integrated-as
> -else
> +ifeq ($(LLVM_IAS),0)
>  CLANG_FLAGS    += -no-integrated-as
>  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> +else
> +CLANG_FLAGS    += -integrated-as
>  endif
>  CLANG_FLAGS    += -Werror=unknown-warning-option
>  KBUILD_CFLAGS  += $(CLANG_FLAGS)
>
> base-commit: d7a86429dbc691bf540688fcc8542cc20246a85b
> prerequisite-patch-id: 0d3072ecb5fd06ff6fd6ea81fe601f6c54c23910
> prerequisite-patch-id: 2654829756eb8a094a0ffad1679caa75a4d86619
> prerequisite-patch-id: a51e7885ca2376d008bbf146a5589da247806f7b
> --
> 2.32.0.554.ge1b32706d8-goog
>


When we negate a flag that is enabled by default,
which is a common way?
 - set it to '0'
 - set is to empty


So, I was wondering if we should support
not only LLVM_IAS=0 but also LLVM_IAS=.

What do you think?


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
  2021-08-05 15:15 ` Masahiro Yamada
@ 2021-08-05 15:18   ` Khem Raj
  2021-08-05 18:40   ` Nick Desaulniers
  1 sibling, 0 replies; 8+ messages in thread
From: Khem Raj @ 2021-08-05 15:18 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nick Desaulniers, Nathan Chancellor, Michal Marek,
	Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	clang-built-linux, Linux Kbuild mailing list,
	open list:DOCUMENTATION, Linux Kernel Mailing List,
	open list:SIFIVE DRIVERS

On Thu, Aug 5, 2021 at 8:16 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > LLVM_IAS=1 controls enabling clang's integrated assembler via
> > -integrated-as. This was an explicit opt in until we could enable
> > assembler support in Clang for more architecures. Now we have support
> > and CI coverage of LLVM_IAS=1 for all architecures except a few more
> > bugs affecting s390 and powerpc.
> >
> > This commit flips the default from opt in via LLVM_IAS=1 to opt out via
> > LLVM_IAS=0.  CI systems or developers that were previously doing builds
> > with CC=clang or LLVM=1 without explicitly setting LLVM_IAS must now
> > explicitly opt out via LLVM_IAS=0, otherwise they will be implicitly
> > opted-in.
> >
> > This finally shortens the command line invocation when cross compiling
> > with LLVM to simply:
> >
> > $ make ARCH=arm64 LLVM=1
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1434
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> > Note: base is:
> > https://lore.kernel.org/lkml/20210802183910.1802120-1-ndesaulniers@google.com/
> >
> >  Documentation/kbuild/llvm.rst | 14 ++++++++------
> >  Makefile                      |  2 +-
> >  arch/riscv/Makefile           |  2 +-
> >  scripts/Makefile.clang        |  6 +++---
> >  4 files changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> > index f8a360958f4c..16712fab4d3a 100644
> > --- a/Documentation/kbuild/llvm.rst
> > +++ b/Documentation/kbuild/llvm.rst
> > @@ -60,17 +60,14 @@ They can be enabled individually. The full list of the parameters: ::
> >           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
> >           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
> >
> > -Currently, the integrated assembler is disabled by default. You can pass
> > -``LLVM_IAS=1`` to enable it.
> > +Currently, the integrated assembler is enabled by default. You can pass
> > +``LLVM_IAS=0`` to disable it.
> >
> >  Omitting CROSS_COMPILE
> >  ----------------------
> >
> >  As explained above, ``CROSS_COMPILE`` is used to set ``--target=<triple>``.
> >
> > -Unless ``LLVM_IAS=1`` is specified, ``CROSS_COMPILE`` is also used to derive
> > -``--prefix=<path>`` to search for the GNU assembler and linker.
> > -
> >  If ``CROSS_COMPILE`` is not specified, the ``--target=<triple>`` is inferred
> >  from ``ARCH``.
> >
> > @@ -78,7 +75,12 @@ That means if you use only LLVM tools, ``CROSS_COMPILE`` becomes unnecessary.
> >
> >  For example, to cross-compile the arm64 kernel::
> >
> > -       make ARCH=arm64 LLVM=1 LLVM_IAS=1
> > +       make ARCH=arm64 LLVM=1
> > +
> > +If ``LLVM_IAS=0`` is specified, ``CROSS_COMPILE`` is also used to derive
> > +``--prefix=<path>`` to search for the GNU assembler and linker. ::
> > +
> > +       make ARCH=arm64 LLVM=1 LLVM_IAS=0 CROSS_COMPILE=aarch64-linux-gnu-
> >
> >  Supported Architectures
> >  -----------------------
> > diff --git a/Makefile b/Makefile
> > index 444558e62cbc..b24b48c9ebb7 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -845,7 +845,7 @@ else
> >  DEBUG_CFLAGS   += -g
> >  endif
> >
> > -ifneq ($(LLVM_IAS),1)
> > +ifeq ($(LLVM_IAS),0)
> >  KBUILD_AFLAGS  += -Wa,-gdwarf-2
> >  endif
> >
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index bc74afdbf31e..807f7c94bc6f 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -41,7 +41,7 @@ endif
> >  ifeq ($(CONFIG_LD_IS_LLD),y)
> >         KBUILD_CFLAGS += -mno-relax
> >         KBUILD_AFLAGS += -mno-relax
> > -ifneq ($(LLVM_IAS),1)
> > +ifeq ($(LLVM_IAS),0)
> >         KBUILD_CFLAGS += -Wa,-mno-relax
> >         KBUILD_AFLAGS += -Wa,-mno-relax
> >  endif
>
>
>
> Please drop these two hunks.
>
> I will apply my patch instead.
> https://lore.kernel.org/patchwork/patch/1472580/
>
>
>
>
>
>
> > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > index 1f4e3eb70f88..3ae63bd35582 100644
> > --- a/scripts/Makefile.clang
> > +++ b/scripts/Makefile.clang
> > @@ -22,12 +22,12 @@ else
> >  CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
> >  endif # CROSS_COMPILE
> >
> > -ifeq ($(LLVM_IAS),1)
> > -CLANG_FLAGS    += -integrated-as
> > -else
> > +ifeq ($(LLVM_IAS),0)
> >  CLANG_FLAGS    += -no-integrated-as
> >  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> >  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> > +else
> > +CLANG_FLAGS    += -integrated-as
> >  endif
> >  CLANG_FLAGS    += -Werror=unknown-warning-option
> >  KBUILD_CFLAGS  += $(CLANG_FLAGS)
> >
> > base-commit: d7a86429dbc691bf540688fcc8542cc20246a85b
> > prerequisite-patch-id: 0d3072ecb5fd06ff6fd6ea81fe601f6c54c23910
> > prerequisite-patch-id: 2654829756eb8a094a0ffad1679caa75a4d86619
> > prerequisite-patch-id: a51e7885ca2376d008bbf146a5589da247806f7b
> > --
> > 2.32.0.554.ge1b32706d8-goog
> >
>
>
> When we negate a flag that is enabled by default,
> which is a common way?
>  - set it to '0'
>  - set is to empty
>
>
> So, I was wondering if we should support
> not only LLVM_IAS=0 but also LLVM_IAS=.
>
> What do you think?

always asking for 0 or 1 is perhaps better.

>
>
> --
> Best Regards
> Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
  2021-08-05 15:15 ` Masahiro Yamada
  2021-08-05 15:18   ` Khem Raj
@ 2021-08-05 18:40   ` Nick Desaulniers
  2021-08-05 23:48     ` Masahiro Yamada
  1 sibling, 1 reply; 8+ messages in thread
From: Nick Desaulniers @ 2021-08-05 18:40 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Khem Raj, Michal Marek, Jonathan Corbet,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, clang-built-linux,
	Linux Kbuild mailing list, open list:DOCUMENTATION,
	Linux Kernel Mailing List, open list:SIFIVE DRIVERS,
	Matthew Wilcox

On Thu, Aug 5, 2021 at 8:16 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > diff --git a/Makefile b/Makefile
> > index 444558e62cbc..b24b48c9ebb7 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -845,7 +845,7 @@ else
> >  DEBUG_CFLAGS   += -g
> >  endif
> >
> > -ifneq ($(LLVM_IAS),1)
> > +ifeq ($(LLVM_IAS),0)
> >  KBUILD_AFLAGS  += -Wa,-gdwarf-2
> >  endif
> >
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index bc74afdbf31e..807f7c94bc6f 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -41,7 +41,7 @@ endif
> >  ifeq ($(CONFIG_LD_IS_LLD),y)
> >         KBUILD_CFLAGS += -mno-relax
> >         KBUILD_AFLAGS += -mno-relax
> > -ifneq ($(LLVM_IAS),1)
> > +ifeq ($(LLVM_IAS),0)
> >         KBUILD_CFLAGS += -Wa,-mno-relax
> >         KBUILD_AFLAGS += -Wa,-mno-relax
> >  endif
>
>
>
> Please drop these two hunks.
>
> I will apply my patch instead.
> https://lore.kernel.org/patchwork/patch/1472580/

Sure.  Will send a v2 with Matthew's suggestion as well.

> When we negate a flag that is enabled by default,
> which is a common way?
>  - set it to '0'
>  - set is to empty
>
>
> So, I was wondering if we should support
> not only LLVM_IAS=0 but also LLVM_IAS=.
>
> What do you think?

LLVM_IAS= looks weird (so I agree with Khem's response), but if it's
common/expected then maybe if there's a way to write a concise check
for either =<blank> or =0?  I don't feel strongly about how it's
specified to disable the integrated assembler, but let's sort that out
_before_ I send a v2.

How can you tell the difference between `make CC=clang` and `make
CC=clang LLVM_IAS=`? (maybe that doesn't matter here, as either imply
"don't use clang as the assembler when compiling with clang")
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1
  2021-08-05 18:40   ` Nick Desaulniers
@ 2021-08-05 23:48     ` Masahiro Yamada
  0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-05 23:48 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Khem Raj, Michal Marek, Jonathan Corbet,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, clang-built-linux,
	Linux Kbuild mailing list, open list:DOCUMENTATION,
	Linux Kernel Mailing List, open list:SIFIVE DRIVERS,
	Matthew Wilcox

On Fri, Aug 6, 2021 at 3:40 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Thu, Aug 5, 2021 at 8:16 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 444558e62cbc..b24b48c9ebb7 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -845,7 +845,7 @@ else
> > >  DEBUG_CFLAGS   += -g
> > >  endif
> > >
> > > -ifneq ($(LLVM_IAS),1)
> > > +ifeq ($(LLVM_IAS),0)
> > >  KBUILD_AFLAGS  += -Wa,-gdwarf-2
> > >  endif
> > >
> > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > > index bc74afdbf31e..807f7c94bc6f 100644
> > > --- a/arch/riscv/Makefile
> > > +++ b/arch/riscv/Makefile
> > > @@ -41,7 +41,7 @@ endif
> > >  ifeq ($(CONFIG_LD_IS_LLD),y)
> > >         KBUILD_CFLAGS += -mno-relax
> > >         KBUILD_AFLAGS += -mno-relax
> > > -ifneq ($(LLVM_IAS),1)
> > > +ifeq ($(LLVM_IAS),0)
> > >         KBUILD_CFLAGS += -Wa,-mno-relax
> > >         KBUILD_AFLAGS += -Wa,-mno-relax
> > >  endif
> >
> >
> >
> > Please drop these two hunks.
> >
> > I will apply my patch instead.
> > https://lore.kernel.org/patchwork/patch/1472580/
>
> Sure.  Will send a v2 with Matthew's suggestion as well.
>
> > When we negate a flag that is enabled by default,
> > which is a common way?
> >  - set it to '0'
> >  - set is to empty
> >
> >
> > So, I was wondering if we should support
> > not only LLVM_IAS=0 but also LLVM_IAS=.
> >
> > What do you think?
>
> LLVM_IAS= looks weird (so I agree with Khem's response), but if it's
> common/expected then maybe if there's a way to write a concise check
> for either =<blank> or =0?  I don't feel strongly about how it's
> specified to disable the integrated assembler, but let's sort that out
> _before_ I send a v2.
>
> How can you tell the difference between `make CC=clang` and `make
> CC=clang LLVM_IAS=`? (maybe that doesn't matter here, as either imply
> "don't use clang as the assembler when compiling with clang")


$(origin LLVM_IAS) knows the difference.

make CC=clang            ->  $(origin LLVM_IAS) is 'undefined'
make CC=clang LLVM_IAS=  ->  $(origin LLVM_IAS) is 'command line'
LLVM_IAS= make CC=clang  ->  $(origin LLVM_IAS) is 'environment'




The following patch makes both LLVM_IAS= and LLVM_IAS=0
work for disabling the integrated assembler.



@@ -22,6 +22,9 @@ else
 CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
 endif # CROSS_COMPILE

+# The integrated assembler is enabled by default.
+LLVM_IAS ?= 1
+
 ifeq ($(LLVM_IAS),1)
 CLANG_FLAGS    += -integrated-as
 else






But, I am not pretty sure if it is a good idea...

Perhaps, it is better to require LLVM_IAS=0
as Khem mentioned.


Another way for avoiding ambiguity is, perhaps
LLVM_IAS_DISABLE=1 instead of LLVM_IAS=0.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-08-05 23:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 23:43 [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1 Nick Desaulniers
2021-08-03  1:32 ` Matthew Wilcox
2021-08-05 13:59   ` Masahiro Yamada
2021-08-03  1:42 ` Khem Raj
2021-08-05 15:15 ` Masahiro Yamada
2021-08-05 15:18   ` Khem Raj
2021-08-05 18:40   ` Nick Desaulniers
2021-08-05 23:48     ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).