[PATCH] Avoid generating invalid GIMPLE in SRA (PR tree-optimization/37716)

Richard Guenther richard.guenther@gmail.com
Wed Dec 3 10:55:00 GMT 2008


On Wed, Dec 3, 2008 at 2:20 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> In sra_build_assignment for scalar_bitfield_p src there are so many places
> that can create invalid GIMPLE that it is IMHO better just to build
> everything as trees (which is done anyway) and gimplify it all together.
>
> Other options are e.g. doing push_gimplify_context early there and
> use gimplify_assign instead of all the
> gimple_build_assign/gimple_seq_add_stmt pairs, or verifying by hand whether
> it is valid GIMPLE and if not, force into temporary.  But the following
> appears to create fewest temporaries from what I've tried.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2008-12-03  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/37716
>        * tree-sra.c (sra_build_assignment): For scalar bitfield SRC construct
>        all the needed operations as trees and gimplify_assign it to dst.
>
>        * g++.dg/torture/pr37716.C: New test.
>
> --- gcc/tree-sra.c.jj   2008-11-05 11:21:13.000000000 +0100
> +++ gcc/tree-sra.c      2008-12-03 00:38:46.000000000 +0100
> @@ -2158,9 +2158,10 @@ sra_build_assignment (tree dst, tree src
>   if (scalar_bitfield_p (src))
>     {
>       tree var, shift, width;
> -      tree utype, stype, stmp, utmp, dtmp;
> +      tree utype, stype;
>       bool unsignedp = (INTEGRAL_TYPE_P (TREE_TYPE (src))
>                        ? TYPE_UNSIGNED (TREE_TYPE (src)) : true);
> +      struct gimplify_ctx gctx;
>
>       var = TREE_OPERAND (src, 0);
>       width = TREE_OPERAND (src, 1);
> @@ -2191,28 +2192,15 @@ sra_build_assignment (tree dst, tree src
>       else if (!TYPE_UNSIGNED (utype))
>        utype = unsigned_type_for (utype);
>
> -      stmp = make_rename_temp (stype, "SR");
> -
>       /* Convert the base var of the BIT_FIELD_REF to the scalar type
>         we use for computation if we cannot use it directly.  */
> -      if (!useless_type_conversion_p (stype, TREE_TYPE (var)))
> -       {
> -         if (INTEGRAL_TYPE_P (TREE_TYPE (var)))
> -           stmt = gimple_build_assign (stmp, fold_convert (stype, var));
> -         else
> -           stmt = gimple_build_assign (stmp, fold_build1 (VIEW_CONVERT_EXPR,
> -                                                          stype, var));
> -         gimple_seq_add_stmt (&seq, stmt);
> -         var = stmp;
> -       }
> +      if (INTEGRAL_TYPE_P (TREE_TYPE (var)))
> +       var = fold_convert (stype, var);
> +      else
> +       var = fold_build1 (VIEW_CONVERT_EXPR, stype, var);
>
>       if (!integer_zerop (shift))
> -       {
> -         stmt = gimple_build_assign (stmp, fold_build2 (RSHIFT_EXPR, stype,
> -                                                        var, shift));
> -         gimple_seq_add_stmt (&seq, stmt);
> -         var = stmp;
> -       }
> +       var = fold_build2 (RSHIFT_EXPR, stype, var, shift);
>
>       /* If we need a masking operation, produce one.  */
>       if (TREE_INT_CST_LOW (width) == TYPE_PRECISION (stype))
> @@ -2222,24 +2210,11 @@ sra_build_assignment (tree dst, tree src
>          tree one = build_int_cst_wide (stype, 1, 0);
>          tree mask = int_const_binop (LSHIFT_EXPR, one, width, 0);
>          mask = int_const_binop (MINUS_EXPR, mask, one, 0);
> -
> -         stmt = gimple_build_assign (stmp, fold_build2 (BIT_AND_EXPR, stype,
> -                                                        var, mask));
> -         gimple_seq_add_stmt (&seq, stmt);
> -         var = stmp;
> +         var = fold_build2 (BIT_AND_EXPR, stype, var, mask);
>        }
>
>       /* After shifting and masking, convert to the target type.  */
> -      utmp = stmp;
> -      if (!useless_type_conversion_p (utype, stype))
> -       {
> -         utmp = make_rename_temp (utype, "SR");
> -
> -         stmt = gimple_build_assign (utmp, fold_convert (utype, var));
> -         gimple_seq_add_stmt (&seq, stmt);
> -
> -         var = utmp;
> -       }
> +      var = fold_convert (utype, var);
>
>       /* Perform sign extension, if required.
>         ???  This should never be necessary.  */
> @@ -2250,40 +2225,29 @@ sra_build_assignment (tree dst, tree src
>                                          size_binop (MINUS_EXPR, width,
>                                                      bitsize_int (1)), 0);
>
> -         stmt = gimple_build_assign (utmp, fold_build2 (BIT_XOR_EXPR, utype,
> -                                                        var, signbit));
> -         gimple_seq_add_stmt (&seq, stmt);
> -
> -         stmt = gimple_build_assign (utmp, fold_build2 (MINUS_EXPR, utype,
> -                                                        utmp, signbit));
> -         gimple_seq_add_stmt (&seq, stmt);
> -
> -         var = utmp;
> +         var = fold_build2 (BIT_XOR_EXPR, utype, var, signbit);
> +         var = fold_build2 (MINUS_EXPR, utype, var, signbit);
>        }
>
>       /* fold_build3 (BIT_FIELD_REF, ...) sometimes returns a cast.  */
>       STRIP_NOPS (dst);
>
>       /* Finally, move and convert to the destination.  */
> -      if (!useless_type_conversion_p (TREE_TYPE (dst), TREE_TYPE (var)))
> -       {
> -         if (INTEGRAL_TYPE_P (TREE_TYPE (dst)))
> -           var = fold_convert (TREE_TYPE (dst), var);
> -         else
> -           var = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (dst), var);
> +      if (INTEGRAL_TYPE_P (TREE_TYPE (dst)))
> +       var = fold_convert (TREE_TYPE (dst), var);
> +      else
> +       var = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (dst), var);
>
> -         /* If the destination is not a register the conversion needs
> -            to be a separate statement.  */
> -         if (!is_gimple_reg (dst))
> -           {
> -             dtmp = make_rename_temp (TREE_TYPE (dst), "SR");
> -             stmt = gimple_build_assign (dtmp, var);
> -             gimple_seq_add_stmt (&seq, stmt);
> -             var = dtmp;
> -           }
> -       }
> -      stmt = gimple_build_assign (dst, var);
> -      gimple_seq_add_stmt (&seq, stmt);
> +      push_gimplify_context (&gctx);
> +      gctx.into_ssa = true;
> +      gctx.allow_rhs_cond_expr = true;
> +
> +      gimplify_assign (dst, var, &seq);
> +
> +      if (gimple_referenced_vars (cfun))
> +       for (var = gctx.temps; var; var = TREE_CHAIN (var))
> +         add_referenced_var (var);
> +      pop_gimplify_context (NULL);
>
>       return seq;
>     }
> --- gcc/testsuite/g++.dg/torture/pr37716.C.jj   2008-12-03 00:42:12.000000000 +0100
> +++ gcc/testsuite/g++.dg/torture/pr37716.C      2008-12-03 00:40:47.000000000 +0100
> @@ -0,0 +1,56 @@
> +// PR tree-optimization/37716
> +// { dg-do compile }
> +
> +struct A
> +{
> +  struct B
> +  {
> +    int a, b, c, d;
> +    void *e[1];
> +  };
> +  B *d;
> +  inline void **f1 (int i) const
> +  {
> +    return d->e + d->c + i;
> +  }
> +};
> +
> +template <typename T>
> +struct C
> +{
> +  struct D
> +  {
> +    void *v;
> +    inline T & f3 ()
> +    {
> +      return *reinterpret_cast <T *> (this);
> +    }
> +  };
> +  union
> +  {
> +    A p;
> +    A::B *d;
> +  };
> +  T & operator[](int i)
> +  {
> +    if (d->a != 1)
> +      f2 ();
> +    return reinterpret_cast <D *> (p.f1 (i))->f3 ();
> +  }
> +  void f2 ();
> +  void f3 (int i, const T & t);
> +};
> +
> +class E
> +{
> +  int e, f;
> +};
> +
> +C <E> c;
> +
> +void
> +foo (int x)
> +{
> +  E e = c[x];
> +  c.f3 (x, e);
> +}
>
>        Jakub
>



More information about the Gcc-patches mailing list