On Fri, Sep 28, 2018 at 7:01 PM Eric Botcazou <ebotca...@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 <ebotca...@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
