Christophe Lyon <christophe.l...@linaro.org> writes:
>> This PR was caused by mismatched expectations between
>> vectorizable_comparison and SLP. We had a "<" comparison
>> between two booleans that were leaves of the SLP tree, so
>> vectorizable_comparison fell back on:
>>
>> /* Invariant comparison. */
>> if (!vectype)
>> {
>> vectype = get_vectype_for_scalar_type (vinfo, TREE_TYPE (rhs1),
>> slp_node);
>> if (maybe_ne (TYPE_VECTOR_SUBPARTS (vectype), nunits))
>> return false;
>> }
>>
>> rhs1 and rhs2 were *unsigned* boolean types, so we got back a vector
>> of unsigned integers. This in itself was OK, and meant that "<"
>> worked as expected without the need for the boolean fix-ups:
>>
>> /* Boolean values may have another representation in vectors
>> and therefore we prefer bit operations over comparison for
>> them (which also works for scalar masks). We store opcodes
>> to use in bitop1 and bitop2. Statement is vectorized as
>> BITOP2 (rhs1 BITOP1 rhs2) or
>> rhs1 BITOP2 (BITOP1 rhs2)
>> depending on bitop1 and bitop2 arity. */
>> bool swap_p = false;
>> if (VECTOR_BOOLEAN_TYPE_P (vectype))
>> {
>>
>> However, vectorizable_comparison then used vect_get_slp_defs to get
>> the actual operands. The request went to vect_get_constant_vectors,
>> which also has logic to calculate the vector type. The problem was
>> that this type was different from the one chosen above:
>>
>> if (VECT_SCALAR_BOOLEAN_TYPE_P (TREE_TYPE (op))
>> && vect_mask_constant_operand_p (stmt_vinfo))
>> vector_type = truth_type_for (stmt_vectype);
>> else
>> vector_type = get_vectype_for_scalar_type (vinfo, TREE_TYPE (op),
>> op_node);
>>
>> So the function gave back a vector of mask types, which here are vectors
>> of *signed* booleans. This meant that "<" gave:
>>
>> true (-1) < false (0)
>>
>> and so the boolean fixup above was needed after all.
>>
>> Fixed by making vectorizable_comparison also pick a mask type in this case.
>>
>> Tested on aarch64-linux-gnu (with and without SVE) and x86_64-linux-gnu.
>> Approved by Richard in the PR.
>>
>> Richard
>>
>>
>> 2020-04-23 Richard Sandiford <richard.sandif...@arm.com>
>>
>> gcc/
>> PR tree-optimization/94727
>> * tree-vect-stmts.c (vectorizable_comparison): Use mask_type when
>> comparing invariant scalar booleans.
>>
>> gcc/testsuite/
>> PR tree-optimization/94727
>> * gcc.dg/vect/pr94727.c: New test.
>
> Hi Richard,
>
> The new testcase fails at execution on arm (and s390 according to
> gcc-testresults).
>
> Can you check?
Thanks for the heads-up. Looks like we need the same fix when
handling comparisons embedded in a VEC_COND_EXPR.
Here too, the problem is that vect_get_constant_vectors will
calculate its own vector type, using truth_type_for on the
STMT_VINFO_VECTYPE, and the vectoriable_* routines need to be
consistent with that.
Tested on arm-linux-gnueabihf and x86_64-linux-gnu. OK to install?
Richard
2020-04-27 Richard Sandiford <richard.sandif...@arm.com>
gcc/
PR tree-optimization/94727
* tree-vect-stmts.c (vect_is_simple_cond): If both comparison
operands are invariant booleans, use the mask type associated with the
STMT_VINFO_VECTYPE. Use !slp_node instead of !vectype to exclude SLP.
(vectorizable_condition): Pass vectype unconditionally to
vect_is_simple_cond.
---
gcc/tree-vect-stmts.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 88a1e2c51d2..1984787bac4 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -9942,16 +9942,21 @@ vect_is_simple_cond (tree cond, vec_info *vinfo,
slp_tree slp_node,
if (! *comp_vectype)
{
tree scalar_type = TREE_TYPE (lhs);
- /* If we can widen the comparison to match vectype do so. */
- if (INTEGRAL_TYPE_P (scalar_type)
- && vectype
- && tree_int_cst_lt (TYPE_SIZE (scalar_type),
- TYPE_SIZE (TREE_TYPE (vectype))))
- scalar_type = build_nonstandard_integer_type
- (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (vectype))),
- TYPE_UNSIGNED (scalar_type));
- *comp_vectype = get_vectype_for_scalar_type (vinfo, scalar_type,
- slp_node);
+ if (VECT_SCALAR_BOOLEAN_TYPE_P (scalar_type))
+ *comp_vectype = truth_type_for (vectype);
+ else
+ {
+ /* If we can widen the comparison to match vectype do so. */
+ if (INTEGRAL_TYPE_P (scalar_type)
+ && !slp_node
+ && tree_int_cst_lt (TYPE_SIZE (scalar_type),
+ TYPE_SIZE (TREE_TYPE (vectype))))
+ scalar_type = build_nonstandard_integer_type
+ (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (vectype))),
+ TYPE_UNSIGNED (scalar_type));
+ *comp_vectype = get_vectype_for_scalar_type (vinfo, scalar_type,
+ slp_node);
+ }
}
return true;
@@ -10066,7 +10071,7 @@ vectorizable_condition (stmt_vec_info stmt_info,
gimple_stmt_iterator *gsi,
else_clause = gimple_assign_rhs3 (stmt);
if (!vect_is_simple_cond (cond_expr, stmt_info->vinfo, slp_node,
- &comp_vectype, &dts[0], slp_node ? NULL : vectype)
+ &comp_vectype, &dts[0], vectype)
|| !comp_vectype)
return false;
--
2.17.1
