Re: [patch] Fix PR tree-optimization/86659

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Sep 28, 2018 at 7:01 PM Eric Botcazou <ebotcazou@adacore.com> wrote:
>
> Hi,
>
> this is a regression introduced by the canonicalization of BIT_FIELD_REF in
> match.pd, which totally disregards the REF_REVERSE_STORAGE_ORDER flag, and
> visible as the failure of gnat.dg/sso/q[23].adb on SPARC 64-bit.  But the
> underlying issue of the missing propagation of the flag during GIMPLE folding
> has probably been latent for quite some time on all active branches.
>
> Tested on x86-64/Linux and SPARC/Solaris, OK for mainline?  And branches?

@@ -853,7 +857,10 @@ gimple_simplify (gimple *stmt, gimple_ma
                op0 = do_valueize (op0, top_valueize, valueized);
                res_op->set_op (code, type, op0,
                                TREE_OPERAND (rhs1, 1),
-                               TREE_OPERAND (rhs1, 2));
+                               TREE_OPERAND (rhs1, 2),
+                               REF_REVERSE_STORAGE_ORDER (rhs1));
+               if (res_op->reverse)
+                 return valueized;
                return (gimple_resimplify3 (seq, res_op, valueize)
                        || valueized);
              }

so the fix is to simply not optimize here?  Are there correctness issues
with the patterns we have for rev-storage?  But then some cases are let through
via the realpart/imagpart/v_c_e case?  I suppose we should never see
REF_REVERSE_STORAGE_ORDER on refs operating on registers (SSA_NAMEs
or even is_gimple_reg()s)?

Note that I think you need to adjust the GENERIC side as well, for example:

static tree
generic_simplify_BIT_FIELD_REF (location_t ARG_UNUSED (loc), enum
tree_code ARG_UNUSED (code), const tree ARG_UNUSED (type), tree op0,
tree op1, tree op2)
{
...
    case VIEW_CONVERT_EXPR:
      {
        tree o20 = TREE_OPERAND (op0, 0);
        {
/* #line 4690 "/tmp/trunk2/gcc/match.pd" */
          tree captures[3] ATTRIBUTE_UNUSED = { o20, op1, op2 };
          if (__builtin_expect (dump_file && (dump_flags &
TDF_FOLDING), 0)) fprintf (dump_file, "Applying pattern %s:%d,
%s:%d\n", "match.pd", 4690, __FILE__, __LINE__);
          tree res_op0;
          res_op0 = captures[0];
          tree res_op1;
          res_op1 = captures[1];
          tree res_op2;
          res_op2 = captures[2];
          tree res;
          res = fold_build3_loc (loc, BIT_FIELD_REF, type, res_op0,
res_op1, res_op2);

where we lose the reverse-storage attribute as well.  You'd probably
have to cut out
rev-storage refs somewhere in genmatch.c.

Richard.

>
> 2018-09-28  Eric Botcazou  <ebotcazou@adacore.com>
>
>         PR tree-optimization/86659
>         * gimple-match.h (struct gimple_match_op): Add reverse field.
>         (gimple_match_op::set_op): New overloaded method.
>         * gimple-match-head.c (maybe_build_generic_op) <BIT_FIELD_REF>: Set
>         the REF_REVERSE_STORAGE_ORDER flag on the value.
>         (gimple_simplify) <GIMPLE_ASSIGN>: For BIT_FIELD_REF, propagate the
>         REF_REVERSE_STORAGE_ORDER flag and avoid simplifying if it is set.
>
> --
> Eric Botcazou