On Tue, Jul 12, 2016 at 09:10:54AM -0500, Segher Boessenkool wrote:
> Hi Marek,
>
> On Mon, Jul 11, 2016 at 09:59:39PM +0200, Marek Polacek wrote:
> > 2016-07-11 Marek Polacek <pola...@redhat.com>
> >
> > PR c/7652
> > * config/rs6000/rs6000.c (rs6000_builtin_vectorized_libmass): Likewise.
>
> Likewise? Like what? :-)
Yikes, I goofed this up when spliting the big patch.
> > --- 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.
Yep, I'm working on this.
> > @@ -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.
Great.
> > @@ -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.
The next version of the warning should recognize this scenario and shouldn't
warn, thus no change will be needed.
> > --- 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.
Yeah, or it at least should have a comment.
> 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.
The next version of the warning should be significantly less verbose, because
it will take "falls through" comments into account.
Thanks.
Marek
