Implement -Wswitch-fallthrough: rs6000

[Marek Polacek]

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  <polacek@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