2022-09-02 10:15:45

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 1/2] powerpc/math_emu/efp: Include module.h

From: Nathan Chancellor <[email protected]>

When building with a recent version of clang, there are a couple of
errors around the call to module_init():

arch/powerpc/math-emu/math_efp.c:927:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
module_init(spe_mathemu_init);
^
int
arch/powerpc/math-emu/math_efp.c:927:13: error: a parameter list without types is only allowed in a function definition
module_init(spe_mathemu_init);
^
2 errors generated.

module_init() is a macro, which is not getting expanded because module.h
is not included in this file. Add the include so that the macro can
expand properly, clearing up the build failure.

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Fixes: ac6f120369ff ("powerpc/85xx: Workaroudn e500 CPU erratum A005")
[chleroy: added fixes tag]
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/math-emu/math_efp.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c
index 39b84e7452e1..aa3bb8da1cb9 100644
--- a/arch/powerpc/math-emu/math_efp.c
+++ b/arch/powerpc/math-emu/math_efp.c
@@ -17,6 +17,7 @@

#include <linux/types.h>
#include <linux/prctl.h>
+#include <linux/module.h>

#include <linux/uaccess.h>
#include <asm/reg.h>
--
2.37.1


2022-09-02 10:26:50

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 2/2] powerpc/math-emu: Remove -w build flag and fix warnings

As reported by Nathan, the module_init() macro was not taken into
account because the header was missing. That means spe_mathemu_init()
was never called.

This should have been detected by gcc at build time, but due to
'-w' flag it went undetected.

Removing that flag leads to many warnings hence errors.

Fix those warnings then remove the -w flag.

Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/math-emu/Makefile | 2 --
arch/powerpc/math-emu/math.c | 18 +++++-----
arch/powerpc/math-emu/math_efp.c | 57 +++++++++++++++++---------------
3 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/math-emu/Makefile b/arch/powerpc/math-emu/Makefile
index a8794032f15f..26fef2e5672e 100644
--- a/arch/powerpc/math-emu/Makefile
+++ b/arch/powerpc/math-emu/Makefile
@@ -16,5 +16,3 @@ obj-$(CONFIG_SPE) += math_efp.o

CFLAGS_fabs.o = -fno-builtin-fabs
CFLAGS_math.o = -fno-builtin-fabs
-
-ccflags-y = -w
diff --git a/arch/powerpc/math-emu/math.c b/arch/powerpc/math-emu/math.c
index 36761bd00f38..936a9a149037 100644
--- a/arch/powerpc/math-emu/math.c
+++ b/arch/powerpc/math-emu/math.c
@@ -24,9 +24,9 @@ FLOATFUNC(mtfsf);
FLOATFUNC(mtfsfi);

#ifdef CONFIG_MATH_EMULATION_HW_UNIMPLEMENTED
-#undef FLOATFUNC(x)
+#undef FLOATFUNC
#define FLOATFUNC(x) static inline int x(void *op1, void *op2, void *op3, \
- void *op4) { }
+ void *op4) { return 0; }
#endif

FLOATFUNC(fadd);
@@ -396,28 +396,28 @@ do_mathemu(struct pt_regs *regs)

case XCR:
op0 = (void *)&regs->ccr;
- op1 = (void *)((insn >> 23) & 0x7);
+ op1 = (void *)(long)((insn >> 23) & 0x7);
op2 = (void *)&current->thread.TS_FPR((insn >> 16) & 0x1f);
op3 = (void *)&current->thread.TS_FPR((insn >> 11) & 0x1f);
break;

case XCRL:
op0 = (void *)&regs->ccr;
- op1 = (void *)((insn >> 23) & 0x7);
- op2 = (void *)((insn >> 18) & 0x7);
+ op1 = (void *)(long)((insn >> 23) & 0x7);
+ op2 = (void *)(long)((insn >> 18) & 0x7);
break;

case XCRB:
- op0 = (void *)((insn >> 21) & 0x1f);
+ op0 = (void *)(long)((insn >> 21) & 0x1f);
break;

case XCRI:
- op0 = (void *)((insn >> 23) & 0x7);
- op1 = (void *)((insn >> 12) & 0xf);
+ op0 = (void *)(long)((insn >> 23) & 0x7);
+ op1 = (void *)(long)((insn >> 12) & 0xf);
break;

case XFLB:
- op0 = (void *)((insn >> 17) & 0xff);
+ op0 = (void *)(long)((insn >> 17) & 0xff);
op1 = (void *)&current->thread.TS_FPR((insn >> 11) & 0x1f);
break;

diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c
index aa3bb8da1cb9..f01e3475f689 100644
--- a/arch/powerpc/math-emu/math_efp.c
+++ b/arch/powerpc/math-emu/math_efp.c
@@ -219,6 +219,7 @@ int do_spe_mathemu(struct pt_regs *regs)
case AB:
case XCR:
FP_UNPACK_SP(SA, va.wp + 1);
+ fallthrough;
case XB:
FP_UNPACK_SP(SB, vb.wp + 1);
break;
@@ -227,8 +228,8 @@ int do_spe_mathemu(struct pt_regs *regs)
break;
}

- pr_debug("SA: %ld %08lx %ld (%ld)\n", SA_s, SA_f, SA_e, SA_c);
- pr_debug("SB: %ld %08lx %ld (%ld)\n", SB_s, SB_f, SB_e, SB_c);
+ pr_debug("SA: %d %08x %d (%d)\n", SA_s, SA_f, SA_e, SA_c);
+ pr_debug("SB: %d %08x %d (%d)\n", SB_s, SB_f, SB_e, SB_c);

switch (func) {
case EFSABS:
@@ -279,7 +280,7 @@ int do_spe_mathemu(struct pt_regs *regs)
} else {
SB_e += (func == EFSCTSF ? 31 : 32);
FP_TO_INT_ROUND_S(vc.wp[1], SB, 32,
- (func == EFSCTSF));
+ (func == EFSCTSF) ? 1 : 0);
}
goto update_regs;

@@ -288,7 +289,7 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_CLEAR_EXCEPTIONS;
FP_UNPACK_DP(DB, vb.dp);

- pr_debug("DB: %ld %08lx %08lx %ld (%ld)\n",
+ pr_debug("DB: %d %08x %08x %d (%d)\n",
DB_s, DB_f1, DB_f0, DB_e, DB_c);

FP_CONV(S, D, 1, 2, SR, DB);
@@ -302,7 +303,7 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_ROUND_S(vc.wp[1], SB, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
goto update_regs;

@@ -313,7 +314,7 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_S(vc.wp[1], SB, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
goto update_regs;

@@ -323,7 +324,7 @@ int do_spe_mathemu(struct pt_regs *regs)
break;

pack_s:
- pr_debug("SR: %ld %08lx %ld (%ld)\n", SR_s, SR_f, SR_e, SR_c);
+ pr_debug("SR: %d %08x %d (%d)\n", SR_s, SR_f, SR_e, SR_c);

FP_PACK_SP(vc.wp + 1, SR);
goto update_regs;
@@ -347,6 +348,7 @@ int do_spe_mathemu(struct pt_regs *regs)
case AB:
case XCR:
FP_UNPACK_DP(DA, va.dp);
+ fallthrough;
case XB:
FP_UNPACK_DP(DB, vb.dp);
break;
@@ -355,9 +357,9 @@ int do_spe_mathemu(struct pt_regs *regs)
break;
}

- pr_debug("DA: %ld %08lx %08lx %ld (%ld)\n",
+ pr_debug("DA: %d %08x %08x %d (%d)\n",
DA_s, DA_f1, DA_f0, DA_e, DA_c);
- pr_debug("DB: %ld %08lx %08lx %ld (%ld)\n",
+ pr_debug("DB: %d %08x %08x %d (%d)\n",
DB_s, DB_f1, DB_f0, DB_e, DB_c);

switch (func) {
@@ -409,7 +411,7 @@ int do_spe_mathemu(struct pt_regs *regs)
} else {
DB_e += (func == EFDCTSF ? 31 : 32);
FP_TO_INT_ROUND_D(vc.wp[1], DB, 32,
- (func == EFDCTSF));
+ (func == EFDCTSF) ? 1 : 0);
}
goto update_regs;

@@ -418,7 +420,7 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_CLEAR_EXCEPTIONS;
FP_UNPACK_SP(SB, vb.wp + 1);

- pr_debug("SB: %ld %08lx %ld (%ld)\n",
+ pr_debug("SB: %d %08x %d (%d)\n",
SB_s, SB_f, SB_e, SB_c);

FP_CONV(D, S, 2, 1, DR, SB);
@@ -432,7 +434,7 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_D(vc.dp[0], DB, 64,
- ((func & 0x1) == 0));
+ ((func & 0x1) == 0) ? 1 : 0);
}
goto update_regs;

@@ -443,7 +445,7 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_ROUND_D(vc.wp[1], DB, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
goto update_regs;

@@ -454,7 +456,7 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_D(vc.wp[1], DB, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
goto update_regs;

@@ -464,7 +466,7 @@ int do_spe_mathemu(struct pt_regs *regs)
break;

pack_d:
- pr_debug("DR: %ld %08lx %08lx %ld (%ld)\n",
+ pr_debug("DR: %d %08x %08x %d (%d)\n",
DR_s, DR_f1, DR_f0, DR_e, DR_c);

FP_PACK_DP(vc.dp, DR);
@@ -493,6 +495,7 @@ int do_spe_mathemu(struct pt_regs *regs)
case XCR:
FP_UNPACK_SP(SA0, va.wp);
FP_UNPACK_SP(SA1, va.wp + 1);
+ fallthrough;
case XB:
FP_UNPACK_SP(SB0, vb.wp);
FP_UNPACK_SP(SB1, vb.wp + 1);
@@ -503,13 +506,13 @@ int do_spe_mathemu(struct pt_regs *regs)
break;
}

- pr_debug("SA0: %ld %08lx %ld (%ld)\n",
+ pr_debug("SA0: %d %08x %d (%d)\n",
SA0_s, SA0_f, SA0_e, SA0_c);
- pr_debug("SA1: %ld %08lx %ld (%ld)\n",
+ pr_debug("SA1: %d %08x %d (%d)\n",
SA1_s, SA1_f, SA1_e, SA1_c);
- pr_debug("SB0: %ld %08lx %ld (%ld)\n",
+ pr_debug("SB0: %d %08x %d (%d)\n",
SB0_s, SB0_f, SB0_e, SB0_c);
- pr_debug("SB1: %ld %08lx %ld (%ld)\n",
+ pr_debug("SB1: %d %08x %d (%d)\n",
SB1_s, SB1_f, SB1_e, SB1_c);

switch (func) {
@@ -568,7 +571,7 @@ int do_spe_mathemu(struct pt_regs *regs)
} else {
SB0_e += (func == EVFSCTSF ? 31 : 32);
FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32,
- (func == EVFSCTSF));
+ (func == EVFSCTSF) ? 1 : 0);
}
if (SB1_c == FP_CLS_NAN) {
vc.wp[1] = 0;
@@ -576,7 +579,7 @@ int do_spe_mathemu(struct pt_regs *regs)
} else {
SB1_e += (func == EVFSCTSF ? 31 : 32);
FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32,
- (func == EVFSCTSF));
+ (func == EVFSCTSF) ? 1 : 0);
}
goto update_regs;

@@ -587,14 +590,14 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
if (SB1_c == FP_CLS_NAN) {
vc.wp[1] = 0;
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
goto update_regs;

@@ -605,14 +608,14 @@ int do_spe_mathemu(struct pt_regs *regs)
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_S(vc.wp[0], SB0, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
if (SB1_c == FP_CLS_NAN) {
vc.wp[1] = 0;
FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
FP_TO_INT_S(vc.wp[1], SB1, 32,
- ((func & 0x3) != 0));
+ ((func & 0x3) != 0) ? 1 : 0);
}
goto update_regs;

@@ -622,9 +625,9 @@ int do_spe_mathemu(struct pt_regs *regs)
break;

pack_vs:
- pr_debug("SR0: %ld %08lx %ld (%ld)\n",
+ pr_debug("SR0: %d %08x %d (%d)\n",
SR0_s, SR0_f, SR0_e, SR0_c);
- pr_debug("SR1: %ld %08lx %ld (%ld)\n",
+ pr_debug("SR1: %d %08x %d (%d)\n",
SR1_s, SR1_f, SR1_e, SR1_c);

FP_PACK_SP(vc.wp, SR0);
--
2.37.1

2022-09-02 15:55:57

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] powerpc/math-emu: Remove -w build flag and fix warnings

Hi Christophe,

On Fri, Sep 02, 2022 at 12:08:55PM +0200, Christophe Leroy wrote:
> As reported by Nathan, the module_init() macro was not taken into
> account because the header was missing. That means spe_mathemu_init()
> was never called.
>
> This should have been detected by gcc at build time, but due to
> '-w' flag it went undetected.
>
> Removing that flag leads to many warnings hence errors.
>
> Fix those warnings then remove the -w flag.
>
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Christophe Leroy <[email protected]>

Thanks for figuring out what was going on here! I took this patch for a
spin with clang and it has a few more errors around
-Wimplicit-fallthrough:

arch/powerpc/math-emu/fctiw.c:18:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_D(r, B, 32, 1);
^
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_ZERO: \
^
arch/powerpc/math-emu/fctiw.c:18:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_NAN: \
^
2 errors generated.
arch/powerpc/math-emu/fctiwz.c:23:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_D(r, B, 32, 1);
^
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_ZERO: \
^
arch/powerpc/math-emu/fctiwz.c:23:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_NAN: \
^
2 errors generated.
make[3]: *** [scripts/Makefile.build:249: arch/powerpc/math-emu/fctiw.o] Error 1
make[3]: *** [scripts/Makefile.build:249: arch/powerpc/math-emu/fctiwz.o] Error 1
arch/powerpc/math-emu/math_efp.c:282:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_S(vc.wp[1], SB, 32,
^
./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S'
#define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:305:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_S(vc.wp[1], SB, 32,
^
./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S'
#define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:316:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_S(vc.wp[1], SB, 32,
^
./include/math-emu/single.h:109:34: note: expanded from macro 'FP_TO_INT_S'
#define FP_TO_INT_S(r,X,rsz,rsg) _FP_TO_INT(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_ZERO: \
^
arch/powerpc/math-emu/math_efp.c:316:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
./include/math-emu/single.h:109:34: note: expanded from macro 'FP_TO_INT_S'
#define FP_TO_INT_S(r,X,rsz,rsg) _FP_TO_INT(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:413:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_D(vc.wp[1], DB, 32,
^
./include/math-emu/double.h:121:40: note: expanded from macro 'FP_TO_INT_ROUND_D'
#define FP_TO_INT_ROUND_D(r,X,rsz,rsg) _FP_TO_INT_ROUND(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:436:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_D(vc.dp[0], DB, 64,
^
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_ZERO: \
^
arch/powerpc/math-emu/math_efp.c:436:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:447:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_D(vc.wp[1], DB, 32,
^
./include/math-emu/double.h:121:40: note: expanded from macro 'FP_TO_INT_ROUND_D'
#define FP_TO_INT_ROUND_D(r,X,rsz,rsg) _FP_TO_INT_ROUND(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:458:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_D(vc.wp[1], DB, 32,
^
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_ZERO: \
^
arch/powerpc/math-emu/math_efp.c:458:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D'
#define FP_TO_INT_D(r,X,rsz,rsg) _FP_TO_INT(D,2,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:573:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32,
^
./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S'
#define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:581:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32,
^
./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S'
#define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:592:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32,
^
./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S'
#define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:599:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32,
^
./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S'
#define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:610:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_S(vc.wp[0], SB0, 32,
^
./include/math-emu/single.h:109:34: note: expanded from macro 'FP_TO_INT_S'
#define FP_TO_INT_S(r,X,rsz,rsg) _FP_TO_INT(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_ZERO: \
^
arch/powerpc/math-emu/math_efp.c:610:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
./include/math-emu/single.h:109:34: note: expanded from macro 'FP_TO_INT_S'
#define FP_TO_INT_S(r,X,rsz,rsg) _FP_TO_INT(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_NAN: \
^
arch/powerpc/math-emu/math_efp.c:617:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
FP_TO_INT_S(vc.wp[1], SB1, 32,
^
./include/math-emu/single.h:109:34: note: expanded from macro 'FP_TO_INT_S'
#define FP_TO_INT_S(r,X,rsz,rsg) _FP_TO_INT(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_ZERO: \
^
arch/powerpc/math-emu/math_efp.c:617:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
./include/math-emu/single.h:109:34: note: expanded from macro 'FP_TO_INT_S'
#define FP_TO_INT_S(r,X,rsz,rsg) _FP_TO_INT(S,1,r,X,rsz,rsg)
^
./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT'
case FP_CLS_NAN: \
^
18 errors generated.

The following diff resolves it and does not introduce any new issues
with GCC. Would you mind squashing it in for a v2? With that:

Reviewed-by: Nathan Chancellor <[email protected]>

Cheers,
Nathan

diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h
index 4b57bbba588a..ae73a30bf1a0 100644
--- a/include/math-emu/op-common.h
+++ b/include/math-emu/op-common.h
@@ -662,12 +662,14 @@ do { \
if (X##_e < 0) \
{ \
FP_SET_EXCEPTION(FP_EX_INEXACT); \
+ fallthrough; \
case FP_CLS_ZERO: \
r = 0; \
} \
else if (X##_e >= rsize - (rsigned > 0 || X##_s) \
|| (!rsigned && X##_s)) \
{ /* overflow */ \
+ fallthrough; \
case FP_CLS_NAN: \
case FP_CLS_INF: \
if (rsigned == 2) \
@@ -767,6 +769,7 @@ do { \
if (X##_e >= rsize - (rsigned > 0 || X##_s) \
|| (!rsigned && X##_s)) \
{ /* overflow */ \
+ fallthrough; \
case FP_CLS_NAN: \
case FP_CLS_INF: \
if (!rsigned) \

2022-09-02 16:14:19

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] powerpc/math-emu: Remove -w build flag and fix warnings

On Fri, Sep 02, 2022 at 08:37:23AM -0700, Nathan Chancellor wrote:
> On Fri, Sep 02, 2022 at 12:08:55PM +0200, Christophe Leroy wrote:
> > This should have been detected by gcc at build time, but due to
> > '-w' flag it went undetected.
> >
> > Removing that flag leads to many warnings hence errors.

> Thanks for figuring out what was going on here! I took this patch for a
> spin with clang and it has a few more errors around
> -Wimplicit-fallthrough:

Maybe add -Wno-implicit-fallthrough? This code is a copy from outside
the kernel, no one has ever wanted to maintain it, if nothing else (the
more politically correct formulation is "we cannot as easily pick up
improvements from upstream if we modify stuff").


Segher

2022-09-02 16:50:04

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] powerpc/math-emu: Remove -w build flag and fix warnings



Le 02/09/2022 à 17:59, Segher Boessenkool a écrit :
> On Fri, Sep 02, 2022 at 08:37:23AM -0700, Nathan Chancellor wrote:
>> On Fri, Sep 02, 2022 at 12:08:55PM +0200, Christophe Leroy wrote:
>>> This should have been detected by gcc at build time, but due to
>>> '-w' flag it went undetected.
>>>
>>> Removing that flag leads to many warnings hence errors.
>
>> Thanks for figuring out what was going on here! I took this patch for a
>> spin with clang and it has a few more errors around
>> -Wimplicit-fallthrough:
>
> Maybe add -Wno-implicit-fallthrough? This code is a copy from outside
> the kernel, no one has ever wanted to maintain it, if nothing else (the
> more politically correct formulation is "we cannot as easily pick up
> improvements from upstream if we modify stuff").
>

There are already such changes in that common file, see for instance
commit f336a009f8e3 ("math-emu: Fix fall-through warning"), was in July
2021.

Christophe

2022-09-02 16:55:13

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] powerpc/math-emu: Remove -w build flag and fix warnings

On Fri, Sep 02, 2022 at 10:59:54AM -0500, Segher Boessenkool wrote:
> On Fri, Sep 02, 2022 at 08:37:23AM -0700, Nathan Chancellor wrote:
> > On Fri, Sep 02, 2022 at 12:08:55PM +0200, Christophe Leroy wrote:
> > > This should have been detected by gcc at build time, but due to
> > > '-w' flag it went undetected.
> > >
> > > Removing that flag leads to many warnings hence errors.
>
> > Thanks for figuring out what was going on here! I took this patch for a
> > spin with clang and it has a few more errors around
> > -Wimplicit-fallthrough:
>
> Maybe add -Wno-implicit-fallthrough? This code is a copy from outside
> the kernel, no one has ever wanted to maintain it, if nothing else (the
> more politically correct formulation is "we cannot as easily pick up
> improvements from upstream if we modify stuff").

Sure, we could do something like this if you preferred:

diff --git a/arch/powerpc/math-emu/Makefile b/arch/powerpc/math-emu/Makefile
index 26fef2e5672e..ed775747a2a5 100644
--- a/arch/powerpc/math-emu/Makefile
+++ b/arch/powerpc/math-emu/Makefile
@@ -16,3 +16,7 @@ obj-$(CONFIG_SPE) += math_efp.o

CFLAGS_fabs.o = -fno-builtin-fabs
CFLAGS_math.o = -fno-builtin-fabs
+
+ifdef CONFIG_CC_IS_CLANG
+ccflags-remove-y := $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
+endif

At the same time, I see other modifications to these files that appear
to be for the kernel only so I suspect that this is already in the "we
cannot as easily pick up improvements from upstream" category,
regardless of that diff. No strong opinion from me, although I see
Christophe already included my suggestion in the most recent series:

https://lore.kernel.org/2663961738a46073713786d4efeb53100ca156e7.1662134272.git.christophe.leroy@csgroup.eu/

Cheers,
Nathan