This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Implement -Wswitch-fallthrough: rs6000
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 12 Jul 2016 09:10:54 -0500
- Subject: Re: Implement -Wswitch-fallthrough: rs6000
- Authentication-results: sourceware.org; auth=none
- References: <20160711194340.GI13963@redhat.com> <20160711195939.GD13963@redhat.com>
Hi Marek,
On Mon, Jul 11, 2016 at 09:59:39PM +0200, Marek Polacek wrote:
> 2016-07-11 Marek Polacek <polacek@redhat.com>
>
> PR c/7652
> * config/rs6000/rs6000.c (rs6000_builtin_vectorized_libmass): Likewise.
Likewise? Like what? :-)
> --- gcc/gcc/config/rs6000/rs6000.c
> +++ gcc/gcc/config/rs6000/rs6000.c
> @@ -5459,6 +5459,7 @@ rs6000_builtin_vectorized_libmass (combined_fn fn, tree type_out,
> CASE_CFN_POW:
> n_args = 2;
> /* fall through */
> + gcc_fallthrough ();
As mentioned elsewhere, please remove comments saying just "fall through"
when adding the new statement.
> @@ -14398,6 +14401,8 @@ altivec_expand_ld_builtin (tree exp, rtx target, bool *expandedp)
> break;
> case ALTIVEC_BUILTIN_LD_INTERNAL_2di:
> icode = CODE_FOR_vector_altivec_load_v2di;
> + /* XXX Really? */
> + gcc_fallthrough ();
> case ALTIVEC_BUILTIN_LD_INTERNAL_1ti:
> icode = CODE_FOR_vector_altivec_load_v1ti;
> break;
Yeah, that's a bug obviously. We'll fix it.
> @@ -30191,6 +30200,7 @@ rs6000_adjust_cost (rtx_insn *insn, rtx link, rtx_insn *dep_insn, int cost)
> && (INSN_CODE (dep_insn) >= 0)
> && (get_attr_type (dep_insn) == TYPE_MFFGPR))
> return 2;
> + gcc_fallthrough ();
>
> default:
> break;
Better to put an extra "break" here. That is usually true if the next
statement (after one or more labels) is a break.
> --- gcc/gcc/config/rs6000/rs6000.md
> +++ gcc/gcc/config/rs6000/rs6000.md
> @@ -8094,6 +8094,8 @@
> case 0:
> if (TARGET_STRING)
> return \"stswi %1,%P0,16\";
> + /* XXX Really fallthru? */
> + gcc_fallthrough ();
> case 1:
> return \"#\";
Yes, really :-) This code is probably better written without a switch
statement at all, but oh well.
I like the new warning, and I'd like to see it in -Wextra. It finds real
problems that are easy to correct. It does trigger a lot on existing code,
but -Wextra is not -Wall.
Segher