Hi all,
Eric reported that builds of LLVM with [1] (close to tip of tree) have CONFIG_AS_HAS_OPTION_ARCH=n because the test for expected failure on invalid input has started succeeding.
This Kconfig test was added because '.option arch' only causes an assembler warning when it is unsupported, rather than a hard error, which is what users of as-instr expect when something is unsupported.
This can be resolved by turning assembler warnings into errors with '-Wa,--fatal-warnings' like we do with the compiler with '-Werror', which is what the first patch does. The second patch removes the invalid test, as the valid test is good enough with fatal warnings.
I have diffed several configurations for the different architectures that use as-instr and I have found no issues.
I think this could go in through either the kbuild or RISC-V tree with sufficient acks but I will let them fight over who takes it :)
[1]: https://github.com/llvm/llvm-project/commit/3ac9fe69f70a2b3541266daedbaaa7dc...
--- Nathan Chancellor (2): kbuild: Add -Wa,--fatal-warnings to as-instr invocation RISC-V: Drop invalid test from CONFIG_AS_HAS_OPTION_ARCH
arch/riscv/Kconfig | 1 - scripts/Kconfig.include | 2 +- scripts/Makefile.compiler | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) --- base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d change-id: 20240124-fix-riscv-option-arch-llvm-18-3cbe7b09a216
Best regards,
Certain assembler instruction tests may only induce warnings from the assembler on an unsupported instruction or option, which causes as-instr to succeed when it was expected to fail. Some tests workaround this limitation by additionally testing that invalid input fails as expected. However, this is fragile if the assembler is changed to accept the invalid input, as it will cause the instruction/option to be unavailable like it was unsupported even when it is.
Use '-Wa,--fatal-warnings' in the as-instr macro to turn these warnings into hard errors, which avoids this fragility and makes tests more robust and well formed.
Cc: stable@vger.kernel.org Suggested-by: Eric Biggers ebiggers@kernel.org Signed-off-by: Nathan Chancellor nathan@kernel.org --- scripts/Kconfig.include | 2 +- scripts/Makefile.compiler | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
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) diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 8fcb427405a6..92be0c9a13ee 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -38,7 +38,7 @@ as-option = $(call try-run,\ # Usage: aflags-y += $(call as-instr,instr,option1,option2)
as-instr = $(call try-run,\ - printf "%b\n" "$(1)" | $(CC) -Werror $(CLANG_FLAGS) $(KBUILD_AFLAGS) -c -x assembler-with-cpp -o "$$TMP" -,$(2),$(3)) + printf "%b\n" "$(1)" | $(CC) -Werror $(CLANG_FLAGS) $(KBUILD_AFLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o "$$TMP" -,$(2),$(3))
# __cc-option # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
Commit e4bb020f3dbb ("riscv: detect assembler support for .option arch") added two tests, one for a valid value to '.option arch' that should succeed and one for an invalid value that is expected to fail to make sure that support for '.option arch' is properly detected because Clang does not error when '.option arch' is not supported:
$ clang --target=riscv64-linux-gnu -Werror -x assembler -c -o /dev/null <(echo '.option arch, +m') /dev/fd/63:1:9: warning: unknown option, expected 'push', 'pop', 'rvc', 'norvc', 'relax' or 'norelax' .option arch, +m ^ $ echo $? 0
Unfortunately, the invalid test started being accepted by Clang after the linked llvm-project change, which causes CONFIG_AS_HAS_OPTION_ARCH and configurations that depend on it to be silently disabled, even though those versions do support '.option arch'.
The invalid test can be avoided altogether by using '-Wa,--fatal-warnings', which will turn all assembler warnings into errors, like '-Werror' does for the compiler:
$ clang --target=riscv64-linux-gnu -Werror -Wa,--fatal-warnings -x assembler -c -o /dev/null <(echo '.option arch, +m') /dev/fd/63:1:9: error: unknown option, expected 'push', 'pop', 'rvc', 'norvc', 'relax' or 'norelax' .option arch, +m ^ $ echo $? 1
The as-instr macros have been updated to make use of this flag, so remove the invalid test, which allows CONFIG_AS_HAS_OPTION_ARCH to work for all compiler versions.
Cc: stable@vger.kernel.org Fixes: e4bb020f3dbb ("riscv: detect assembler support for .option arch") Link: https://github.com/llvm/llvm-project/commit/3ac9fe69f70a2b3541266daedbaaa7dc... Reported-by: Eric Biggers ebiggers@kernel.org Closes: https://lore.kernel.org/r/20240121011341.GA97368@sol.localdomain/ Signed-off-by: Nathan Chancellor nathan@kernel.org --- arch/riscv/Kconfig | 1 - 1 file changed, 1 deletion(-)
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"
On Thu, Jan 25, 2024 at 10:32:10AM -0700, Nathan Chancellor wrote:
Hi all,
Eric reported that builds of LLVM with [1] (close to tip of tree) have CONFIG_AS_HAS_OPTION_ARCH=n because the test for expected failure on invalid input has started succeeding.
This Kconfig test was added because '.option arch' only causes an assembler warning when it is unsupported, rather than a hard error, which is what users of as-instr expect when something is unsupported.
This can be resolved by turning assembler warnings into errors with '-Wa,--fatal-warnings' like we do with the compiler with '-Werror', which is what the first patch does. The second patch removes the invalid test, as the valid test is good enough with fatal warnings.
I have diffed several configurations for the different architectures that use as-instr and I have found no issues.
I think this could go in through either the kbuild or RISC-V tree with sufficient acks but I will let them fight over who takes it :)
Nathan Chancellor (2): kbuild: Add -Wa,--fatal-warnings to as-instr invocation RISC-V: Drop invalid test from CONFIG_AS_HAS_OPTION_ARCH
arch/riscv/Kconfig | 1 - scripts/Kconfig.include | 2 +- scripts/Makefile.compiler | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-)
Looks good,
Tested-by: Eric Biggers ebiggers@google.com
Unfortunately another LLVM commit just broke TOOLCHAIN_HAS_VECTOR_CRYPTO, so I've sent out a patch to fix that too...
But with all the fixes applied it works again.
- Eric
On Sat, Jan 27, 2024 at 5:03 PM Eric Biggers ebiggers@kernel.org wrote:
On Thu, Jan 25, 2024 at 10:32:10AM -0700, Nathan Chancellor wrote:
Hi all,
Eric reported that builds of LLVM with [1] (close to tip of tree) have CONFIG_AS_HAS_OPTION_ARCH=n because the test for expected failure on invalid input has started succeeding.
This Kconfig test was added because '.option arch' only causes an assembler warning when it is unsupported, rather than a hard error, which is what users of as-instr expect when something is unsupported.
This can be resolved by turning assembler warnings into errors with '-Wa,--fatal-warnings' like we do with the compiler with '-Werror', which is what the first patch does. The second patch removes the invalid test, as the valid test is good enough with fatal warnings.
I have diffed several configurations for the different architectures that use as-instr and I have found no issues.
I think this could go in through either the kbuild or RISC-V tree with sufficient acks but I will let them fight over who takes it :)
Nathan Chancellor (2): kbuild: Add -Wa,--fatal-warnings to as-instr invocation RISC-V: Drop invalid test from CONFIG_AS_HAS_OPTION_ARCH
arch/riscv/Kconfig | 1 - scripts/Kconfig.include | 2 +- scripts/Makefile.compiler | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-)
Looks good,
Tested-by: Eric Biggers ebiggers@google.com
Unfortunately another LLVM commit just broke TOOLCHAIN_HAS_VECTOR_CRYPTO, so I've sent out a patch to fix that too...
But with all the fixes applied it works again.
- Eric
For this series
Tested-by: Andy Chiu andybnac@gmail.com Reviewed-by: Andy Chiu andybnac@gmail.com
Thanks, Andy
On Thu, Jan 25, 2024 at 10:32:10AM -0700, Nathan Chancellor wrote:
Hi all,
Eric reported that builds of LLVM with [1] (close to tip of tree) have CONFIG_AS_HAS_OPTION_ARCH=n because the test for expected failure on invalid input has started succeeding.
This Kconfig test was added because '.option arch' only causes an assembler warning when it is unsupported, rather than a hard error, which is what users of as-instr expect when something is unsupported.
This can be resolved by turning assembler warnings into errors with '-Wa,--fatal-warnings' like we do with the compiler with '-Werror', which is what the first patch does. The second patch removes the invalid test, as the valid test is good enough with fatal warnings.
I have diffed several configurations for the different architectures that use as-instr and I have found no issues.
I think this could go in through either the kbuild or RISC-V tree with sufficient acks but I will let them fight over who takes it :)
I think RISC-V would be good, since building the vector crypto stuff also needs this fix.
Tested-by: Conor Dooley conor.dooley@microchip.com Reviewed-by: Conor Dooley conor.dooley@microchip.com
Cheers, Conor.
On Tue, Jan 30, 2024 at 1:34 AM Conor Dooley conor@kernel.org wrote:
On Thu, Jan 25, 2024 at 10:32:10AM -0700, Nathan Chancellor wrote:
Hi all,
Eric reported that builds of LLVM with [1] (close to tip of tree) have CONFIG_AS_HAS_OPTION_ARCH=n because the test for expected failure on invalid input has started succeeding.
This Kconfig test was added because '.option arch' only causes an assembler warning when it is unsupported, rather than a hard error, which is what users of as-instr expect when something is unsupported.
This can be resolved by turning assembler warnings into errors with '-Wa,--fatal-warnings' like we do with the compiler with '-Werror', which is what the first patch does. The second patch removes the invalid test, as the valid test is good enough with fatal warnings.
I have diffed several configurations for the different architectures that use as-instr and I have found no issues.
I think this could go in through either the kbuild or RISC-V tree with sufficient acks but I will let them fight over who takes it :)
I think RISC-V would be good, since building the vector crypto stuff also needs this fix.
OK, then I will give ack instead of applying this series.
Acked-by: Masahiro Yamada masahiroy@kernel.org
Tested-by: Conor Dooley conor.dooley@microchip.com Reviewed-by: Conor Dooley conor.dooley@microchip.com
Hello:
This series was applied to riscv/linux.git (fixes) by Palmer Dabbelt palmer@rivosinc.com:
On Thu, 25 Jan 2024 10:32:10 -0700 you wrote:
Hi all,
Eric reported that builds of LLVM with [1] (close to tip of tree) have CONFIG_AS_HAS_OPTION_ARCH=n because the test for expected failure on invalid input has started succeeding.
This Kconfig test was added because '.option arch' only causes an assembler warning when it is unsupported, rather than a hard error, which is what users of as-instr expect when something is unsupported.
[...]
Here is the summary with links: - [1/2] kbuild: Add -Wa,--fatal-warnings to as-instr invocation https://git.kernel.org/riscv/c/0ee695a471a7 - [2/2] RISC-V: Drop invalid test from CONFIG_AS_HAS_OPTION_ARCH https://git.kernel.org/riscv/c/3aff0c459e77
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org
-
Andy Chiu -
Conor Dooley -
Eric Biggers -
Masahiro Yamada -
Nathan Chancellor -
patchwork-bot+linux-riscv＠kernel.org