On Tue, 31 May 2016, Richard Biener wrote:
>
> The following patch adds missing patterns with swapped comparison
> operands for the @0 < @1 and @0 < @2 to @0 < min (@1, @2) transform.
> This nullifies the difference gimplify-into-SSA made on
> rhfuhf.F:ROFOCK which causes the function no longer to be miscompiled
> (by vectorization eventually, -fno-tree-vectorize also fixes the
> miscompare at r235817).
The following is a variant, avoiding the pattern repetition in match.pd
by (finally) completing a local patch I had to support "commutating"
relational operators. It also adds sanity-checking for what you apply
:c to which catches a few cases in match.pd I introduce :C for in this
patch.
Bootstrap / regtest in progress on x86_64-unknown-linux-gnu.
Richard.
2016-05-31 Richard Biener <rguent...@suse.de>
PR tree-optimization/71311
* genmatch.c (comparison_code_p): New predicate.
(swap_tree_comparison): New function.
(commutate): Add for_vec parameter to append new for entries.
Support commutating relational operators by swapping it alongside
operands.
(lower_commutative): Adjust.
(dt_simplify::gen): Do not pass artificial operators to gen
functions.
(decision_tree::gen): Do not add artificial operators as parameters.
(parser::parse_expr): Verify operator commutativity when :c is
applied. Allow :C to override this.
* match.pd: Adjust patterns to use :C instead of :c where required.
Add :c to @0 < @1 && @0 < @2 -> @0 < min(@1,@2) transform.
Index: gcc/genmatch.c
===================================================================
*** gcc/genmatch.c (revision 236877)
--- gcc/genmatch.c (working copy)
*************** commutative_ternary_tree_code (enum tree
*** 291,296 ****
--- 291,325 ----
return false;
}
+ /* Return true if CODE is a comparison. */
+
+ bool
+ comparison_code_p (enum tree_code code)
+ {
+ switch (code)
+ {
+ case EQ_EXPR:
+ case NE_EXPR:
+ case ORDERED_EXPR:
+ case UNORDERED_EXPR:
+ case LTGT_EXPR:
+ case UNEQ_EXPR:
+ case GT_EXPR:
+ case GE_EXPR:
+ case LT_EXPR:
+ case LE_EXPR:
+ case UNGT_EXPR:
+ case UNGE_EXPR:
+ case UNLT_EXPR:
+ case UNLE_EXPR:
+ return true;
+
+ default:
+ break;
+ }
+ return false;
+ }
+
/* Base class for all identifiers the parser knows. */
*************** get_operator (const char *id, bool allow
*** 528,533 ****
--- 557,598 ----
return operators->find_with_hash (&tem, tem.hashval);
}
+ /* Return the comparison operators that results if the operands are
+ swapped. This is safe for floating-point. */
+
+ id_base *
+ swap_tree_comparison (operator_id *p)
+ {
+ switch (p->code)
+ {
+ case EQ_EXPR:
+ case NE_EXPR:
+ case ORDERED_EXPR:
+ case UNORDERED_EXPR:
+ case LTGT_EXPR:
+ case UNEQ_EXPR:
+ return p;
+ case GT_EXPR:
+ return get_operator ("LT_EXPR");
+ case GE_EXPR:
+ return get_operator ("LE_EXPR");
+ case LT_EXPR:
+ return get_operator ("GT_EXPR");
+ case LE_EXPR:
+ return get_operator ("GE_EXPR");
+ case UNGT_EXPR:
+ return get_operator ("UNLT_EXPR");
+ case UNGE_EXPR:
+ return get_operator ("UNLE_EXPR");
+ case UNLT_EXPR:
+ return get_operator ("UNGT_EXPR");
+ case UNLE_EXPR:
+ return get_operator ("UNGE_EXPR");
+ default:
+ gcc_unreachable ();
+ }
+ }
+
typedef hash_map<nofree_string_hash, unsigned> cid_map_t;
*************** cartesian_product (const vec< vec<operan
*** 816,822 ****
/* Lower OP to two operands in case it is marked as commutative. */
static vec<operand *>
! commutate (operand *op)
{
vec<operand *> ret = vNULL;
--- 881,887 ----
/* Lower OP to two operands in case it is marked as commutative. */
static vec<operand *>
! commutate (operand *op, vec<vec<user_id *> > &for_vec)
{
vec<operand *> ret = vNULL;
*************** commutate (operand *op)
*** 827,833 ****
ret.safe_push (op);
return ret;
}
! vec<operand *> v = commutate (c->what);
for (unsigned i = 0; i < v.length (); ++i)
{
capture *nc = new capture (c->location, c->where, v[i]);
--- 892,898 ----
ret.safe_push (op);
return ret;
}
! vec<operand *> v = commutate (c->what, for_vec);
for (unsigned i = 0; i < v.length (); ++i)
{
capture *nc = new capture (c->location, c->where, v[i]);
*************** commutate (operand *op)
*** 845,851 ****
vec< vec<operand *> > ops_vector = vNULL;
for (unsigned i = 0; i < e->ops.length (); ++i)
! ops_vector.safe_push (commutate (e->ops[i]));
auto_vec< vec<operand *> > result;
auto_vec<operand *> v (e->ops.length ());
--- 910,916 ----
vec< vec<operand *> > ops_vector = vNULL;
for (unsigned i = 0; i < e->ops.length (); ++i)
! ops_vector.safe_push (commutate (e->ops[i], for_vec));
auto_vec< vec<operand *> > result;
auto_vec<operand *> v (e->ops.length ());
*************** commutate (operand *op)
*** 868,873 ****
--- 933,982 ----
for (unsigned i = 0; i < result.length (); ++i)
{
expr *ne = new expr (e);
+ if (operator_id *p = dyn_cast <operator_id *> (ne->operation))
+ {
+ if (comparison_code_p (p->code))
+ ne->operation = swap_tree_comparison (p);
+ }
+ else if (user_id *p = dyn_cast <user_id *> (ne->operation))
+ {
+ bool found_compare = false;
+ for (unsigned j = 0; j < p->substitutes.length (); ++j)
+ if (operator_id *q = dyn_cast <operator_id *> (p->substitutes[j]))
+ {
+ if (comparison_code_p (q->code)
+ && swap_tree_comparison (q) != q)
+ {
+ found_compare = true;
+ break;
+ }
+ }
+ if (found_compare)
+ {
+ user_id *newop = new user_id ("<internal>");
+ for (unsigned j = 0; j < p->substitutes.length (); ++j)
+ {
+ id_base *subst = p->substitutes[j];
+ if (operator_id *q = dyn_cast <operator_id *> (subst))
+ {
+ if (comparison_code_p (q->code))
+ subst = swap_tree_comparison (q);
+ }
+ newop->substitutes.safe_push (subst);
+ }
+ ne->operation = newop;
+ /* Search for 'p' inside the for vector and push 'newop'
+ to the same level. */
+ for (unsigned j = 0; newop && j < for_vec.length (); ++j)
+ for (unsigned k = 0; k < for_vec[j].length (); ++k)
+ if (for_vec[j][k] == p)
+ {
+ for_vec[j].safe_push (newop);
+ newop = NULL;
+ break;
+ }
+ }
+ }
ne->is_commutative = false;
// result[i].length () is 2 since e->operation is binary
for (unsigned j = result[i].length (); j; --j)
*************** commutate (operand *op)
*** 884,890 ****
static void
lower_commutative (simplify *s, vec<simplify *>& simplifiers)
{
! vec<operand *> matchers = commutate (s->match);
for (unsigned i = 0; i < matchers.length (); ++i)
{
simplify *ns = new simplify (s->kind, matchers[i], s->result,
--- 993,999 ----
static void
lower_commutative (simplify *s, vec<simplify *>& simplifiers)
{
! vec<operand *> matchers = commutate (s->match, s->for_vec);
for (unsigned i = 0; i < matchers.length (); ++i)
{
simplify *ns = new simplify (s->kind, matchers[i], s->result,
*************** dt_simplify::gen (FILE *f, int indent, b
*** 3248,3254 ****
fprintf_indent (f, indent, "if (%s (res_code, res_ops, seq, "
"valueize, type, captures", info->fname);
for (unsigned i = 0; i < s->for_subst_vec.length (); ++i)
! fprintf (f, ", %s", s->for_subst_vec[i].second->id);
fprintf (f, "))\n");
fprintf_indent (f, indent, " return true;\n");
}
--- 3357,3364 ----
fprintf_indent (f, indent, "if (%s (res_code, res_ops, seq, "
"valueize, type, captures", info->fname);
for (unsigned i = 0; i < s->for_subst_vec.length (); ++i)
! if (s->for_subst_vec[i].first->used)
! fprintf (f, ", %s", s->for_subst_vec[i].second->id);
fprintf (f, "))\n");
fprintf_indent (f, indent, " return true;\n");
}
*************** dt_simplify::gen (FILE *f, int indent, b
*** 3260,3266 ****
fprintf (f, ", op%d", i);
fprintf (f, ", captures");
for (unsigned i = 0; i < s->for_subst_vec.length (); ++i)
! fprintf (f, ", %s", s->for_subst_vec[i].second->id);
fprintf (f, ");\n");
fprintf_indent (f, indent, "if (res) return res;\n");
}
--- 3370,3379 ----
fprintf (f, ", op%d", i);
fprintf (f, ", captures");
for (unsigned i = 0; i < s->for_subst_vec.length (); ++i)
! {
! if (s->for_subst_vec[i].first->used)
! fprintf (f, ", %s", s->for_subst_vec[i].second->id);
! }
fprintf (f, ");\n");
fprintf_indent (f, indent, "if (res) return res;\n");
}
*************** dt_simplify::gen (FILE *f, int indent, b
*** 3269,3274 ****
--- 3382,3389 ----
{
for (unsigned i = 0; i < s->for_subst_vec.length (); ++i)
{
+ if (! s->for_subst_vec[i].first->used)
+ continue;
if (is_a <operator_id *> (s->for_subst_vec[i].second))
fprintf_indent (f, indent, "enum tree_code %s = %s;\n",
s->for_subst_vec[i].first->id,
*************** decision_tree::gen (FILE *f, bool gimple
*** 3425,3430 ****
--- 3540,3547 ----
}
for (unsigned i = 0; i < s->s->s->for_subst_vec.length (); ++i)
{
+ if (! s->s->s->for_subst_vec[i].first->used)
+ continue;
if (is_a <operator_id *> (s->s->s->for_subst_vec[i].second))
fprintf (f, ", enum tree_code ARG_UNUSED (%s)",
s->s->s->for_subst_vec[i].first->id);
*************** parser::parse_expr ()
*** 3885,3890 ****
--- 4002,4031 ----
while (*sp)
{
if (*sp == 'c')
+ {
+ if (operator_id *p
+ = dyn_cast<operator_id *> (e->operation))
+ {
+ if (!commutative_tree_code (p->code)
+ && !comparison_code_p (p->code))
+ fatal_at (token, "operation is not commutative");
+ }
+ else if (user_id *p = dyn_cast<user_id *> (e->operation))
+ for (unsigned i = 0;
+ i < p->substitutes.length (); ++i)
+ {
+ if (operator_id *q
+ = dyn_cast<operator_id *> (p->substitutes[i]))
+ {
+ if (!commutative_tree_code (q->code)
+ && !comparison_code_p (q->code))
+ fatal_at (token, "operation %s is not "
+ "commutative", q->id);
+ }
+ }
+ is_commutative = true;
+ }
+ else if (*sp == 'C')
is_commutative = true;
else if (*sp == 's')
{
Index: gcc/match.pd
===================================================================
*** gcc/match.pd (revision 236877)
--- gcc/match.pd (working copy)
*************** DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
*** 189,195 ****
/* Optimize -A / A to -1.0 if we don't care about
NaNs or Infinities. */
(simplify
! (rdiv:c @0 (negate @0))
(if (FLOAT_TYPE_P (type)
&& ! HONOR_NANS (type)
&& ! HONOR_INFINITIES (type))
--- 189,195 ----
/* Optimize -A / A to -1.0 if we don't care about
NaNs or Infinities. */
(simplify
! (rdiv:C @0 (negate @0))
(if (FLOAT_TYPE_P (type)
&& ! HONOR_NANS (type)
&& ! HONOR_INFINITIES (type))
*************** DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
*** 2811,2817 ****
/* cabs(x+0i) or cabs(0+xi) -> abs(x). */
(simplify
! (CABS (complex:c @0 real_zerop@1))
(abs @0))
/* trunc(trunc(x)) -> trunc(x), etc. */
--- 2833,2839 ----
/* cabs(x+0i) or cabs(0+xi) -> abs(x). */
(simplify
! (CABS (complex:C @0 real_zerop@1))
(abs @0))
/* trunc(trunc(x)) -> trunc(x), etc. */
*************** DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
*** 3246,3252 ****
(for op (lt le gt ge)
ext (min min max max)
(simplify
! (bit_and (op:s @0 @1) (op:s @0 @2))
(if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
(op @0 (ext @1 @2)))))
--- 3268,3274 ----
(for op (lt le gt ge)
ext (min min max max)
(simplify
! (bit_and (op:cs @0 @1) (op:cs @0 @2))
(if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
(op @0 (ext @1 @2)))))
