Re: [PATCH] Fix PR middle-end/81194, ICE during RTL pass: expand

[Richard Biener <richard.guenther@gmail.com>]

On Thu, Jun 29, 2017 at 5:01 AM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> With the fix to PR51513 and follow on fixes for PR80707, PR80775 and PR80823,
> we can now end up with switch statements that contain nothing but a default
> case statement.  The expand_case() function contains code that assumes we
> have at least one non-default case statement, leading to the ICE reported
> in the PR81194.
>
> This patch fixes the bug by expanding switch statements that contain only
> a default case statement, as a GOTO to the default case's label.
>
> This passed bootstrap and regtesting on x86_64-linux with no regressions.
> Ok for trunk?

Ok.

Btw, I'm curious -- which pass ends up with just the default case?  IMHO
it would be a cfgcleanup task but eventually we fail to run it?  That is
cleanup_control_expr_graph calls find_taken_edge () and maybe that
doesn't handle the case where there's just the default label and thus
the value of the switch var doesn't matter ...

Thanks,
Richard.

> Peter
>
> gcc/
>         PR middle-end/81194
>         * cfgexpand.c (expand_gimple_stmt_1): Handle switch statements
>         with only one label.
>         * stmt.c (expand_case): Assert NCASES is greater than one.
>
> gcc/testsuite/
>         PR middle-end/81194
>         * g++.dg/pr81194.C: New test.
>
> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c     (revision 249747)
> +++ gcc/cfgexpand.c     (working copy)
> @@ -3566,7 +3566,13 @@
>      case GIMPLE_PREDICT:
>        break;
>      case GIMPLE_SWITCH:
> -      expand_case (as_a <gswitch *> (stmt));
> +      {
> +       gswitch *swtch = as_a <gswitch *> (stmt);
> +       if (gimple_switch_num_labels (swtch) == 1)
> +         expand_goto (CASE_LABEL (gimple_switch_default_label (swtch)));
> +       else
> +         expand_case (swtch);
> +      }
>        break;
>      case GIMPLE_ASM:
>        expand_asm_stmt (as_a <gasm *> (stmt));
> Index: gcc/stmt.c
> ===================================================================
> --- gcc/stmt.c  (revision 249747)
> +++ gcc/stmt.c  (working copy)
> @@ -1142,8 +1142,11 @@
>    /* cleanup_tree_cfg removes all SWITCH_EXPR with their index
>       expressions being INTEGER_CST.  */
>    gcc_assert (TREE_CODE (index_expr) != INTEGER_CST);
> -
>
> +  /* Optimization of switch statements with only one label has already
> +     occurred, so we should never see them at this point.  */
> +  gcc_assert (ncases > 1);
> +
>    do_pending_stack_adjust ();
>
>    /* Find the default case target label.  */
> Index: gcc/testsuite/g++.dg/pr81194.C
> ===================================================================
> --- gcc/testsuite/g++.dg/pr81194.C      (nonexistent)
> +++ gcc/testsuite/g++.dg/pr81194.C      (working copy)
> @@ -0,0 +1,60 @@
> +// { dg-do compile }
> +// { dg-options "-O2 -std=c++17 -fno-exceptions" }
> +
> +template <class a> struct b { typedef a *c; };
> +class e {};
> +template <typename a> class d {
> +public:
> +  typedef typename b<a>::c c;
> +  c begin();
> +  c end();
> +};
> +struct f {
> +  enum { g } h;
> +};
> +struct i {
> +  d<f *> j();
> +};
> +struct l {
> +  d<i *> k();
> +};
> +class ac;
> +class o {
> +public:
> +  o(int *, int *, int *, ac *);
> +};
> +class ac {
> +public:
> +  ac(e);
> +  virtual o *ae(int *, int *, int *, int *);
> +};
> +class p {
> +  void af(f *m) {
> +    switch (m->h)
> +    case f::g:
> +      ag();
> +  }
> +
> +public:
> +  void n() {
> +    l ah;
> +    for (i *ai : ah.k())
> +      for (f *m : ai->j())
> +        af(m);
> +  }
> +  virtual void ag() { __builtin_unreachable(); }
> +};
> +template <typename = int> class an : o {
> +public:
> +  an(int *, int *, int *, int *, ac *);
> +};
> +class q : ac {
> +public:
> +  q() : ac([]() -> e {}()) {}
> +  o *ae(int *ap, int *aq, int *ar, int *as) { an(ap, aq, ar, as, this); }
> +};
> +template <typename at>
> +an<at>::an(int *, int *aq, int *ar, int *as, ac *au) : o(aq, ar, as, au) {
> +  p().n();
> +}
> +void av() { new q; }
>