2021-08-02 23:44:30

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

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 <[email protected]>
---
Note: base is:
https://lore.kernel.org/lkml/[email protected]/

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



2021-08-03 01:35:09

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

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.


2021-08-03 01:43:53

by Khem Raj

[permalink] [raw]
Subject: Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

On Mon, Aug 2, 2021 at 4:43 PM Nick Desaulniers <[email protected]> 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 <[email protected]>
> ---
> Note: base is:
> https://lore.kernel.org/lkml/[email protected]/
>
> 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
>

2021-08-05 14:47:55

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

On Tue, Aug 3, 2021 at 10:33 AM Matthew Wilcox <[email protected]> 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

2021-08-05 18:45:24

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <[email protected]> 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 <[email protected]>
> ---
> Note: base is:
> https://lore.kernel.org/lkml/[email protected]/
>
> 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

2021-08-05 18:59:03

by Khem Raj

[permalink] [raw]
Subject: Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

On Thu, Aug 5, 2021 at 8:16 AM Masahiro Yamada <[email protected]> wrote:
>
> On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <[email protected]> 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 <[email protected]>
> > ---
> > Note: base is:
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > 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

2021-08-06 00:42:58

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

On Thu, Aug 5, 2021 at 8:16 AM Masahiro Yamada <[email protected]> wrote:
>
> On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <[email protected]> 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

2021-08-06 02:27:42

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] scripts/Makefile.clang: default to LLVM_IAS=1

On Fri, Aug 6, 2021 at 3:40 AM Nick Desaulniers <[email protected]> wrote:
>
> On Thu, Aug 5, 2021 at 8:16 AM Masahiro Yamada <[email protected]> wrote:
> >
> > On Tue, Aug 3, 2021 at 8:43 AM Nick Desaulniers <[email protected]> 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