Re: [tuples] walk_tree for tuples

["Christopher Matthews" <chrismatthews at google dot com>]

So, if labels returns the default labels too, does it makes sense that
nlabels should include the default label in the count?   I was just
using it to iterate though the labels, and it seemed non-intuitive
that I had to add a 1.

On 7/12/07, Aldy Hernandez <aldyh@redhat.com> wrote:
> My apologies Chris.  You're right.  There is no 1 off error.
>
> The problem was that I was assuming gs_switch_*label would return the
> default label with an index of 0.  Diego also agrees this should be the
> case, so I'm reverting my one off changes, but removing the "+ 1" in
> gs_switch_label and gs_switch_set_label.
>
> Can you double check the logic to make sure I didn't botch something up?
>
> > This has been a source of confusion for me, but I *think* there is an
> > extra 1 tree's worth added in because of the wacky memory allocation
> > in the back of:
> >
> > > tree index;
> > > tree GTY ((length ("%h.nlabels + 1"))) labels[1];
>
> Correct.
>
> > so, should it be nlabels + default - the one extra?
>
> Yes.  Fixed.
>
> Sorry about the confusion.
>
> Committing to branch.
>
>         * gimple-ir.c (gs_build_switch_1): Allocate one less tree.
>         (gs_build_switch_1): Offset labels by one.
>         (gs_switch_label): Same.
>         (gs_switch_set_label): Same.
>
> Index: gimple-ir.c
> ===================================================================
> --- gimple-ir.c (revision 126596)
> +++ gimple-ir.c (working copy)
> @@ -437,9 +437,9 @@ gs_build_switch_1 (unsigned int nlabels,
>  {
>    gimple p;
>
> -  /* nlabels + default label.  */
> -  p = ggc_alloc_cleared ( sizeof (struct gimple_statement_switch)
> -                          + (sizeof (tree) * (nlabels + 1)));
> +  /* nlabels + 1 default label - 1 extra from struct.  */
> +  p = ggc_alloc_cleared (sizeof (struct gimple_statement_switch)
> +                         + sizeof (tree) * nlabels);
>    GS_CODE (p) = GS_SWITCH;
>
>    gs_switch_set_nlabels (p, nlabels);
> @@ -475,7 +475,7 @@ gs_build_switch (unsigned int nlabels, t
>  /* Construct a GS_SWITCH statement.
>
>     INDEX is the switch's index.
> -   NLABLES is the number of labels in the switch excluding the default.
> +   NLABELS is the number of labels in the switch excluding the default.
>     ARGS is a vector of labels excluding the default.  */
>
>  gimple
> @@ -486,7 +486,7 @@ gs_build_switch_vec (tree index, tree de
>    gimple p = gs_build_switch_1 (nlabels, index, default_label);
>
>    for (i = 0; i < nlabels; i++)
> -    gs_switch_set_label (p, i, VEC_index (tree, args, i));
> +    gs_switch_set_label (p, i + 1, VEC_index (tree, args, i));
>
>    return p;
>  }
> Index: gimple-ir.h
> ===================================================================
> --- gimple-ir.h (revision 126596)
> +++ gimple-ir.h (working copy)
> @@ -1067,20 +1067,25 @@ gs_switch_set_default_label (gimple gs,
>    gs->gs_switch.labels[0] = label;
>  }
>
> +/* Return the label numbered INDEX.  The default label is 0, followed by any
> +   labels in a switch statement.  */
> +
>  static inline tree
>  gs_switch_label (gimple gs, int index)
>  {
>    GS_CHECK (gs, GS_SWITCH);
>    gcc_assert ((unsigned)index <= gs->gs_switch.nlabels);
> -  return gs->gs_switch.labels[index + 1];
> +  return gs->gs_switch.labels[index];
>  }
>
> +/* Set the label number INDEX to LABEL.  0 is always the default label.  */
> +
>  static inline void
>  gs_switch_set_label (gimple gs, int index, tree label)
>  {
>    GS_CHECK (gs, GS_SWITCH);
>    gcc_assert ((unsigned)index <= gs->gs_switch.nlabels);
> -  gs->gs_switch.labels[index + 1] = label;
> +  gs->gs_switch.labels[index] = label;
>  }
>
>  /* GS_OMP_* accessors. */