Patchwork [5/8] tcg: Remove stray semi-colons from target-*/helper.h

login
register
mail settings
Submitter Richard Henderson
Date Sept. 15, 2013, 12:03 a.m.
Message ID <1379203434-5680-6-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/274993/
State New
Headers show

Comments

Richard Henderson - Sept. 15, 2013, 12:03 a.m.
During GEN_HELPER=1, these are actually stray top-level semi-colons
which are technically invalid ISO C, but GCC accepts as an extension.
If we added enough __extension__ markers that we could dare use
-Wpedantic, we'd see

  warning: ISO C does not allow extra ‘;’ outside of a function

This will become a hard error in the next patch, wherein those ; will
appear in the middle of a data structure.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-alpha/helper.h |  2 +-
 target-arm/helper.h   |  8 ++++----
 target-cris/helper.h  |  8 ++++----
 target-mips/helper.h  | 12 ++++++------
 target-ppc/helper.h   | 10 +++++-----
 target-sparc/helper.h | 18 +++++++++---------
 6 files changed, 29 insertions(+), 29 deletions(-)
Stefan Weil - Sept. 15, 2013, 7:03 a.m.
Am 15.09.2013 02:03, schrieb Richard Henderson:
> During GEN_HELPER=1, these are actually stray top-level semi-colons
> which are technically invalid ISO C, but GCC accepts as an extension.
> If we added enough __extension__ markers that we could dare use
> -Wpedantic, we'd see
>
>   warning: ISO C does not allow extra ‘;’ outside of a function
>
> This will become a hard error in the next patch, wherein those ; will
> appear in the middle of a data structure.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---

Instead of removing the semicolons from the DEF_HELPER_x lines,
I'd prefer removing them from the DEF_HELPER_FLAGS_x definitions.

Code formatters and static code analyzers (maybe humans, too) prefer
lines which look like valid C syntax, therefore

DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64);

is better for such tools than

DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64)

The compiler will also complain if someone adds a new DEF_HELPER_FLAGS_x
without semicolon in the first case, but it won't complain if someone adds
it with semicolon in the second case.

Regards,
Stefan
Peter Maydell - Sept. 15, 2013, 10:44 a.m.
On 15 September 2013 08:03, Stefan Weil <sw@weilnetz.de> wrote:
> Instead of removing the semicolons from the DEF_HELPER_x lines,
> I'd prefer removing them from the DEF_HELPER_FLAGS_x definitions.
>
> Code formatters and static code analyzers (maybe humans, too) prefer
> lines which look like valid C syntax, therefore
>
> DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64);
>
> is better for such tools than
>
> DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64)

I agree it looks nicer, but for this kind of multipurpose macro definition
where one of the redefinitions is used for things like creating a
data structure (eg Richard's patch 6/8 in this series) it just doesn't
work, because the required separator for entries in an array is a comma,
not a semicolon.

It's just an unavoidable cost of doing this with the preprocessor
rather than (say) a custom little language which we parsed with
a python or perl script.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Patch

diff --git a/target-alpha/helper.h b/target-alpha/helper.h
index 732b701..5a0e78c 100644
--- a/target-alpha/helper.h
+++ b/target-alpha/helper.h
@@ -114,7 +114,7 @@  DEF_HELPER_FLAGS_1(tbia, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(tbis, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(tb_flush, TCG_CALL_NO_RWG, void, env)
 
-DEF_HELPER_1(halt, void, i64);
+DEF_HELPER_1(halt, void, i64)
 
 DEF_HELPER_FLAGS_0(get_vmtime, TCG_CALL_NO_RWG, i64)
 DEF_HELPER_FLAGS_0(get_walltime, TCG_CALL_NO_RWG, i64)
diff --git a/target-arm/helper.h b/target-arm/helper.h
index 63ae13a..cac9564 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -247,10 +247,10 @@  DEF_HELPER_3(neon_qshl_u32, i32, env, i32, i32)
 DEF_HELPER_3(neon_qshl_s32, i32, env, i32, i32)
 DEF_HELPER_3(neon_qshl_u64, i64, env, i64, i64)
 DEF_HELPER_3(neon_qshl_s64, i64, env, i64, i64)
-DEF_HELPER_3(neon_qshlu_s8, i32, env, i32, i32);
-DEF_HELPER_3(neon_qshlu_s16, i32, env, i32, i32);
-DEF_HELPER_3(neon_qshlu_s32, i32, env, i32, i32);
-DEF_HELPER_3(neon_qshlu_s64, i64, env, i64, i64);
+DEF_HELPER_3(neon_qshlu_s8, i32, env, i32, i32)
+DEF_HELPER_3(neon_qshlu_s16, i32, env, i32, i32)
+DEF_HELPER_3(neon_qshlu_s32, i32, env, i32, i32)
+DEF_HELPER_3(neon_qshlu_s64, i64, env, i64, i64)
 DEF_HELPER_3(neon_qrshl_u8, i32, env, i32, i32)
 DEF_HELPER_3(neon_qrshl_s8, i32, env, i32, i32)
 DEF_HELPER_3(neon_qrshl_u16, i32, env, i32, i32)
diff --git a/target-cris/helper.h b/target-cris/helper.h
index 8e8365c..0ac31f5 100644
--- a/target-cris/helper.h
+++ b/target-cris/helper.h
@@ -4,14 +4,14 @@  DEF_HELPER_2(raise_exception, void, env, i32)
 DEF_HELPER_2(tlb_flush_pid, void, env, i32)
 DEF_HELPER_2(spc_write, void, env, i32)
 DEF_HELPER_3(dump, void, i32, i32, i32)
-DEF_HELPER_1(rfe, void, env);
-DEF_HELPER_1(rfn, void, env);
+DEF_HELPER_1(rfe, void, env)
+DEF_HELPER_1(rfn, void, env)
 
 DEF_HELPER_3(movl_sreg_reg, void, env, i32, i32)
 DEF_HELPER_3(movl_reg_sreg, void, env, i32, i32)
 
-DEF_HELPER_FLAGS_1(lz, TCG_CALL_NO_SE, i32, i32);
-DEF_HELPER_FLAGS_4(btst, TCG_CALL_NO_SE, i32, env, i32, i32, i32);
+DEF_HELPER_FLAGS_1(lz, TCG_CALL_NO_SE, i32, i32)
+DEF_HELPER_FLAGS_4(btst, TCG_CALL_NO_SE, i32, env, i32, i32, i32)
 
 DEF_HELPER_FLAGS_4(evaluate_flags_muls, TCG_CALL_NO_SE, i32, env, i32, i32, i32)
 DEF_HELPER_FLAGS_4(evaluate_flags_mulu, TCG_CALL_NO_SE, i32, env, i32, i32, i32)
diff --git a/target-mips/helper.h b/target-mips/helper.h
index ed75e2c..1a8b86d 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -148,7 +148,7 @@  DEF_HELPER_2(mtc0_taghi, void, env, tl)
 DEF_HELPER_2(mtc0_datahi, void, env, tl)
 
 /* MIPS MT functions */
-DEF_HELPER_2(mftgpr, tl, env, i32);
+DEF_HELPER_2(mftgpr, tl, env, i32)
 DEF_HELPER_2(mftlo, tl, env, i32)
 DEF_HELPER_2(mfthi, tl, env, i32)
 DEF_HELPER_2(mftacx, tl, env, i32)
@@ -165,11 +165,11 @@  DEF_HELPER_1(evpe, tl, env)
 #endif /* !CONFIG_USER_ONLY */
 
 /* microMIPS functions */
-DEF_HELPER_4(lwm, void, env, tl, tl, i32);
-DEF_HELPER_4(swm, void, env, tl, tl, i32);
+DEF_HELPER_4(lwm, void, env, tl, tl, i32)
+DEF_HELPER_4(swm, void, env, tl, tl, i32)
 #ifdef TARGET_MIPS64
-DEF_HELPER_4(ldm, void, env, tl, tl, i32);
-DEF_HELPER_4(sdm, void, env, tl, tl, i32);
+DEF_HELPER_4(ldm, void, env, tl, tl, i32)
+DEF_HELPER_4(sdm, void, env, tl, tl, i32)
 #endif
 
 DEF_HELPER_2(fork, void, tl, tl)
@@ -615,7 +615,7 @@  DEF_HELPER_FLAGS_4(dmsubu, 0, void, tl, tl, i32, env)
 DEF_HELPER_FLAGS_1(bitrev, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_3(insv, 0, tl, env, tl, tl)
 #if defined(TARGET_MIPS64)
-DEF_HELPER_FLAGS_3(dinsv, 0, tl, env, tl, tl);
+DEF_HELPER_FLAGS_3(dinsv, 0, tl, env, tl, tl)
 #endif
 
 /* DSP Compare-Pick Sub-class insns */
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 56814b5..6d282bb 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -168,8 +168,8 @@  DEF_HELPER_3(vslo, void, avr, avr, avr)
 DEF_HELPER_3(vsro, void, avr, avr, avr)
 DEF_HELPER_3(vaddcuw, void, avr, avr, avr)
 DEF_HELPER_3(vsubcuw, void, avr, avr, avr)
-DEF_HELPER_2(lvsl, void, avr, tl);
-DEF_HELPER_2(lvsr, void, avr, tl);
+DEF_HELPER_2(lvsl, void, avr, tl)
+DEF_HELPER_2(lvsr, void, avr, tl)
 DEF_HELPER_4(vaddsbs, void, env, avr, avr, avr)
 DEF_HELPER_4(vaddshs, void, env, avr, avr, avr)
 DEF_HELPER_4(vaddsws, void, env, avr, avr, avr)
@@ -220,7 +220,7 @@  DEF_HELPER_5(vmsumuhs, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vmsumshm, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vmsumshs, void, env, avr, avr, avr, avr)
 DEF_HELPER_4(vmladduhm, void, avr, avr, avr, avr)
-DEF_HELPER_2(mtvscr, void, env, avr);
+DEF_HELPER_2(mtvscr, void, env, avr)
 DEF_HELPER_3(lvebx, void, env, avr, tl)
 DEF_HELPER_3(lvehx, void, env, avr, tl)
 DEF_HELPER_3(lvewx, void, env, avr, tl)
@@ -349,7 +349,7 @@  DEF_HELPER_2(load_slb_vsid, tl, env, tl)
 DEF_HELPER_FLAGS_1(slbia, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(slbie, TCG_CALL_NO_RWG, void, env, tl)
 #endif
-DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl);
+DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
 DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
 
 DEF_HELPER_FLAGS_1(602_mfrom, TCG_CALL_NO_RWG_SE, tl, tl)
@@ -367,7 +367,7 @@  DEF_HELPER_3(divo, tl, env, tl, tl)
 DEF_HELPER_3(divs, tl, env, tl, tl)
 DEF_HELPER_3(divso, tl, env, tl, tl)
 
-DEF_HELPER_2(load_dcr, tl, env, tl);
+DEF_HELPER_2(load_dcr, tl, env, tl)
 DEF_HELPER_3(store_dcr, void, env, tl, tl)
 
 DEF_HELPER_2(load_dump_spr, void, env, i32)
diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index 15f7328..2a771b2 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -103,7 +103,7 @@  DEF_HELPER_3(fmuls, f32, env, f32, f32)
 DEF_HELPER_3(fdivs, f32, env, f32, f32)
 
 DEF_HELPER_3(fsmuld, f64, env, f32, f32)
-DEF_HELPER_3(fdmulq, void, env, f64, f64);
+DEF_HELPER_3(fdmulq, void, env, f64, f64)
 
 DEF_HELPER_FLAGS_1(fnegs, TCG_CALL_NO_RWG_SE, f32, f32)
 DEF_HELPER_2(fitod, f64, env, s32)
@@ -156,22 +156,22 @@  DEF_HELPER_FLAGS_3(bshuffle, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
     DEF_HELPER_FLAGS_2(f ## name ## 32s, TCG_CALL_NO_RWG_SE, \
                        i32, i32, i32)
 
-VIS_HELPER(padd);
-VIS_HELPER(psub);
+VIS_HELPER(padd)
+VIS_HELPER(psub)
 #define VIS_CMPHELPER(name)                                              \
     DEF_HELPER_FLAGS_2(f##name##16, TCG_CALL_NO_RWG_SE,      \
                        i64, i64, i64)                                    \
     DEF_HELPER_FLAGS_2(f##name##32, TCG_CALL_NO_RWG_SE,      \
                        i64, i64, i64)
-VIS_CMPHELPER(cmpgt);
-VIS_CMPHELPER(cmpeq);
-VIS_CMPHELPER(cmple);
-VIS_CMPHELPER(cmpne);
+VIS_CMPHELPER(cmpgt)
+VIS_CMPHELPER(cmpeq)
+VIS_CMPHELPER(cmple)
+VIS_CMPHELPER(cmpne)
 #endif
 #undef F_HELPER_0_1
 #undef VIS_HELPER
 #undef VIS_CMPHELPER
-DEF_HELPER_1(compute_psr, void, env);
-DEF_HELPER_1(compute_C_icc, i32, env);
+DEF_HELPER_1(compute_psr, void, env)
+DEF_HELPER_1(compute_C_icc, i32, env)
 
 #include "exec/def-helper.h"