Hi,
currently, on trunk, the tests gcc.dg/vect/vect-cond-reduc-1.c and
gcc.dg/pr68286.c fail when compiling for amdgcn-unknown-amdhsa.
The reason seems to lie in the interaction of the changes that have been
introduced by revision r276659
("Allow COND_EXPR and VEC_COND_EXPR condtions to trap" by Ilya Leoshkevich) of
trunk and the vectorized code that is generated for amdgcn.
If the function maybe_resimplify_conditional_op from gimple-match-head.c gets
called on a conditional operation without an "else" part, it
makes the operation unconditional, but only if the operation cannot trap. To
check this, it uses operation_could_trap_p.
This ends up in a violated assertion in the latter function if
maybe_resimplify_conditional_op is called on a COND_EXPR or VEC_COND_EXPR:
/* This function cannot tell whether or not COND_EXPR and VEC_COND_EXPR could
trap, because that depends on the respective condition op. */
gcc_assert (op != COND_EXPR && op != VEC_COND_EXPR);
A related issue has been resolved by the patch that was committed as r276915
("PR middle-end/92063" by Jakub Jelinek).
In our case, the error is triggered by the simplification rule at line 3450 of
gcc/match.pd:
/* A + (B vcmp C ? 1 : 0) -> A - (B vcmp C ? -1 : 0), since vector comparisons
return all -1 or all 0 results. */
/* ??? We could instead convert all instances of the vec_cond to negate,
but that isn't necessarily a win on its own. */
(simplify
(plus:c @3 (view_convert? (vec_cond:s @0 integer_each_onep@1 integer_zerop@2)))
(if (VECTOR_TYPE_P (type)
&& known_eq (TYPE_VECTOR_SUBPARTS (type),
TYPE_VECTOR_SUBPARTS (TREE_TYPE (@1)))
&& (TYPE_MODE (TREE_TYPE (type))
== TYPE_MODE (TREE_TYPE (TREE_TYPE (@1)))))
(minus @3 (view_convert (vec_cond @0:0 (negate @1) @2)))))
)
It seems that this rule is not invoked when compiling for x86_64 where the
generated code for vect-cond-reduc-1.c does not contain anything that would
match this rule. Could it be that there is no test covering this rule for
commonly tested architectures?
I have changed maybe_resimplify_conditional_op to check if a COND_EXPR or
VEC_COND_EXPR could trap by checking whether the condition can trap using
generic_expr_could_trap_p. Judging from the comment above the assertion and the
code changes of r276659, it seems that this is both necessary and
sufficient to verify if those expressions can trap.
Does that sound reasonable and can the patch be included in trunk?
The patch fixes the failing tests for me and does not cause any visible
regressions in the results of "make check" which I have executed for targets
amdgcn-unknown-amdhsa
and x86_64-pc-linux-gnu.
Best regards,
Frederik
2019-11-28 Frederik Harwath <frede...@codesourcery.com>
gcc/
* gimple-match-head.c (maybe_resimplify_conditional_op): use
generic_expr_could_trap_p to check if the condition of COND_EXPR or
VEC_COND_EXPR can trap.
---
gcc/gimple-match-head.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
index 2996bade301..4da6c4d7458 100644
--- a/gcc/gimple-match-head.c
+++ b/gcc/gimple-match-head.c
@@ -144,9 +144,17 @@ maybe_resimplify_conditional_op (gimple_seq *seq,
gimple_match_op *res_op,
/* Likewise if the operation would not trap. */
bool honor_trapv = (INTEGRAL_TYPE_P (res_op->type)
&& TYPE_OVERFLOW_TRAPS (res_op->type));
- if (!operation_could_trap_p ((tree_code) res_op->code,
- FLOAT_TYPE_P (res_op->type),
- honor_trapv, res_op->op_or_null (1)))
+ tree_code op_code = (tree_code) res_op->code;
+ /* COND_EXPR and VEC_COND_EXPR will trap if, and only if, the condition
+ traps and hence we have to check this. For all other operations, we
+ don't need to consider the operands. */
+ bool op_could_trap = op_code == COND_EXPR || op_code == VEC_COND_EXPR ?
+ generic_expr_could_trap_p (res_op->ops[0]) :
+ operation_could_trap_p ((tree_code) res_op->code,
+ FLOAT_TYPE_P (res_op->type),
+ honor_trapv, res_op->op_or_null (1));
+
+ if (!op_could_trap)
{
res_op->cond.cond = NULL_TREE;
return false;
--
2.17.1
