On Wed, Aug 03, 2016 at 10:56:08AM -0600, Jeff Law wrote: > On 07/27/2016 10:53 AM, Marek Polacek wrote: > > These are the cases where I wasn't sure if the falls through were > > intentional > > or not. > > > > This patch has been tested on powerpc64le-unknown-linux-gnu, > > aarch64-linux-gnu, > > and x86_64-redhat-linux. > > > > 2016-07-27 Marek Polacek <pola...@redhat.com> > > > > PR c/7652 > > gcc/ > > * config/i386/i386.c (ix86_expand_branch): Add gcc_fallthrough.. > I'm pretty sure this is an intended fallthru.
Ok. > > * cselib.c (cselib_expand_value_rtx_1): Likewise. > I'm pretty sure this is an intended fallthru. But rather than fallthru, can > we just "return orig;"? If more code gets added here it'll be less and less > clear if we continue to fall thru. Done in the patch I just posted. > > * gensupport.c (get_alternatives_number): Likewise. > > (subst_dup): Likewise. > These are definitely an intended fallthrough. E & V are essentially the > same thing, except that for V the vector may be NULL. Ok. > > * gimplify.c (goa_stabilize_expr): Likewise. > Definitely intended fallthrough. We're "cleverly" handling the 2nd operand > of binary expressions, then falling into the unary expression cases which > handle the 1st operand. Ok. > > * hsa-gen.c (gen_hsa_insn_for_internal_fn_call): Likewise. > I think this is a bug, but please contact Martin Jambor directly on this to > confirm. Done. > > * reg-stack.c (get_true_reg): Likewise. > Pretty sure this is intentional. Like the cselib.c case, I'd prefer to just > duplicate the code from teh fallthru path since it's trivial and makes the > intent clearer. Done. > > * tree-complex.c (expand_complex_division): Likewise. > No sure on this one. Ok, I kept it as it was. > > * tree-data-ref.c (get_references_in_stmt): Likewise. > Intentional fallthru. See how we change ref.is_read for IFN_MASK_LOAD, then > test it in the fallthru path. Ok. > > * tree-pretty-print.c (dump_generic_node): Likewise. > ?!? This looks like a false positive from your warning. We're not falling > into any case statements here AFAICT. I've fixed the warning to not warn here anymore. > > * var-tracking.c (adjust_mems): Likewise. > Definitely an intended fallthru. Though I don't like the way this code is > structured at all. Yea, it's ugly. > > gcc/cp/ > > * call.c (add_builtin_candidate): Add gcc_fallthrough. > > * cxx-pretty-print.c (pp_cxx_unqualified_id): Likewise. > > * parser.c (cp_parser_skip_to_end_of_statement): Likewise. > > (cp_parser_cache_defarg): Likewise. > No idea on these. > > > > gcc/c-family/ > > * c-ada-spec.c (dump_generic_ada_node): Add gcc_fallthrough. > I don't think this is supposed to fall thru. If it falls through then it's > going to issue an error about an unexpected TREE_VEC node when we were > actually working on a TREE_BINFO node. I think a return 0 is appropriate > here. > > > > libcpp/ > > * pch.c (write_macdef): Add CPP_FALLTHRU. > > * lex.c (_cpp_lex_direct): Likewise. > No idea on these. > > Sooo. For those where I've indicated fallthru is intentional, drop the > comment and/or rewrite to avoid fallthru as indicated. Done. > You've got one that looks like a error in your warning > (tree-pretty-print.c). Fixed. > Sync with Martin J. on the hsa warning. I think it's a bug in the hsa code, > but confirm with Martin. Done. > I think the c-ada-spec should be twiddled to avoid the fallthru with a > simple "return 0;" Done. > For the rest, use your patch as-is. That preserves behavior and indicates > we're not sure if fallthru is appropriate or not. For cp/ feel free to ping > Jason directly on those for his thoughts. Thanks a lot for reviewing this. I'll post an updated version later on. Marek