Re: [PATCH -next v21 23/27] riscv: detect assembler support for .option arch

Eric Biggers
2024-01-24T21:58:35Z

On Mon, Jan 22, 2024 at 03:29:18PM -0700, Nathan Chancellor wrote:
> On Sun, Jan 21, 2024 at 10:10:09AM -0800, Eric Biggers wrote:
> > On Sun, Jan 21, 2024 at 10:32:59PM +0800, Andy Chiu wrote:
> > >
> > > Maybe what we really should do is to upgrade the condition check to a
> > > one liner shell script and grep if "Warning" is being printed. Sadly
> > > this warning is not failing the compilation with -Werror.
> > >
> > > I can try forming a patch on this if it feels alright to people.
> >
> > What about -Wa,--fatal-warnings ?
>
> I suspect that would work, the following diff appears to work for me
> with a version of clang that does and does not support '.option arch',
> (although I am not sure if adding -Wa,--fatal-warnings will have any
> other consequences):
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index bffbd869a068..e3142ce531a0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -315,7 +315,6 @@ config AS_HAS_OPTION_ARCH
> # https://reviews.llvm.org/D123515
> def_bool y
> depends on $(as-instr, .option arch$(comma) +m)
> - depends on !$(as-instr, .option arch$(comma) -i)
>
> source "arch/riscv/Kconfig.socs"
> source "arch/riscv/Kconfig.errata"
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 5a84b6443875..3ee8ecfb8c04 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -33,7 +33,7 @@ ld-option = $(success,$(LD) -v $(1))
>
> # $(as-instr,<instr>)
> # Return y if the assembler supports <instr>, n otherwise
> -as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler-with-cpp -o /dev/null -)
> +as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o /dev/null -)
>
> # check if $(CC) and $(LD) exist
> $(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found)

It looks good to me as long as none of the as-instr users turn out to have any
expected warnings.

Maybe send the change to scripts/Kconfig.include as a separate patch?

- Eric