| Message ID | 20230404-mips-unused-named-parameters-v1-1-71d6c656f1de@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 275aca650e76e203e3efc71835d7195e18d2e718 |
| Headers | show |
| Series | MIPS: Drop unused positional parameter in local_irq_{dis,en}able | expand |
On Tue, Apr 04, 2023 at 02:18:41PM -0700, Nathan Chancellor wrote: > When building with clang's integrated assembler, it points out that the > CONFIG_CPU_HAS_DIEI versions of local_irq_enable and local_irq_disable > have a named parameter that is not used in the body of the macro and it > thinks that $8 is a positional parameter, rather than a register: > > arch/mips/include/asm/asmmacro.h:48:2: warning: macro defined with named parameters which are not used in macro body, possible positional parameter found in body which will have no effect > .macro local_irq_enable reg=$8 > ^ > > The comment above the function that performs this check in LLVM notes > that the warning may trigger in this case, even though it is not > problematic. It is easy enough to clean this up by just omitting the > named parameter for this version of the macro, as it is entirely unused. > > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/1415 > Link: https://github.com/llvm/llvm-project/commit/81c944cadb7f9e55b3517b7423a820e2577b9279 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > --- > arch/mips/include/asm/asmmacro.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) applied to mips-next. Thomas.
On Tue, 4 Apr 2023, Nathan Chancellor wrote: > When building with clang's integrated assembler, it points out that the > CONFIG_CPU_HAS_DIEI versions of local_irq_enable and local_irq_disable > have a named parameter that is not used in the body of the macro and it > thinks that $8 is a positional parameter, rather than a register: > > arch/mips/include/asm/asmmacro.h:48:2: warning: macro defined with named > parameters which are not used in macro body, possible positional parameter found > in body which will have no effect > .macro local_irq_enable reg=$8 > ^ > > The comment above the function that performs this check in LLVM notes > that the warning may trigger in this case, even though it is not > problematic. It is easy enough to clean this up by just omitting the > named parameter for this version of the macro, as it is entirely unused. You need to remove the argument from the !CONFIG_CPU_HAS_DIEI versions of the macros then too or otherwise we now have an inconsistent API between kernel configurations. AFAICT the only call site is in arch/mips/kernel/entry.S (and for `local_irq_enable' only), and it doesn't supply the argument, which is why you could have removed it for CONFIG_CPU_HAS_DIEI at all. FWIW I find your reported warning questionable, there are valid reasons, such as the very situation here, why an argument might not be used. It is only a macro after all, effectively a source code transformation tool. Similarly the C preprocessor does not warn about unused macro arguments. So IMHO your change has crippled reasonable code just to paper over a tool's deficiency. Maciej
diff --git a/arch/mips/include/asm/asmmacro.h b/arch/mips/include/asm/asmmacro.h index 1c4438f3f2ab..067a635d3bc8 100644 --- a/arch/mips/include/asm/asmmacro.h +++ b/arch/mips/include/asm/asmmacro.h @@ -45,12 +45,12 @@ #endif #ifdef CONFIG_CPU_HAS_DIEI - .macro local_irq_enable reg=t0 + .macro local_irq_enable ei irq_enable_hazard .endm - .macro local_irq_disable reg=t0 + .macro local_irq_disable di irq_disable_hazard .endm
When building with clang's integrated assembler, it points out that the CONFIG_CPU_HAS_DIEI versions of local_irq_enable and local_irq_disable have a named parameter that is not used in the body of the macro and it thinks that $8 is a positional parameter, rather than a register: arch/mips/include/asm/asmmacro.h:48:2: warning: macro defined with named parameters which are not used in macro body, possible positional parameter found in body which will have no effect .macro local_irq_enable reg=$8 ^ The comment above the function that performs this check in LLVM notes that the warning may trigger in this case, even though it is not problematic. It is easy enough to clean this up by just omitting the named parameter for this version of the macro, as it is entirely unused. Reported-by: Nick Desaulniers <ndesaulniers@google.com> Link: https://github.com/ClangBuiltLinux/linux/issues/1415 Link: https://github.com/llvm/llvm-project/commit/81c944cadb7f9e55b3517b7423a820e2577b9279 Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- arch/mips/include/asm/asmmacro.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- base-commit: 101f26c72825c5dba1dfe826e4202a9a04b435c6 change-id: 20230404-mips-unused-named-parameters-b32df71aadda Best regards,