On Mon, Jan 13, 2020 at 05:30:23PM +0000, Wilco Dijkstra wrote:
> Further improve the ctz recognition: Avoid ICEing on negative shift
> counts or multiply constants. Check the type is 8 bits for the string
> constant case to avoid accidentally matching a wide STRING_CST.
> Add a tree_expr_nonzero_p check to allow the optimization even if
> CTZ_DEFINED_VALUE_AT_ZERO returns 0 or 1. Add extra test cases.
>
> (note the diff uses the old tree and includes Jakub's bootstrap fixes)
You should rebase it because you'll be committing it against trunk
which already has those changes.
> @@ -1879,7 +1879,7 @@ optimize_count_trailing_zeroes (tree array_ref, tree x,
> tree mulc,
> if (!low || !integer_zerop (low))
> return false;
>
> - unsigned shiftval = tree_to_uhwi (tshift);
> + unsigned shiftval = tree_to_shwi (tshift);
This relies on the FEs to narrow the type of say:
x >> (((__uint128_t) 0x12 << 64) | 0x1234567812345678ULL)
> @@ -1889,12 +1889,13 @@ optimize_count_trailing_zeroes (tree array_ref, tree
> x, tree mulc,
> if (!ctor)
> return false;
>
> - unsigned HOST_WIDE_INT val = tree_to_uhwi (mulc);
> + /* Extract the binary representation of the multiply constant. */
> + unsigned HOST_WIDE_INT val = TREE_INT_CST_LOW (mulc);
Will it work properly with the signed types though?
The difference is whether the C code we are matching will use logical or
arithmetic right shift. And if the power of two times mulc ever can have
sign bit set, it will then use negative indexes into the array.
> - if (TREE_CODE (ctor) == STRING_CST)
> + if (TREE_CODE (ctor) == STRING_CST && TYPE_PRECISION (type) == 8)
Isn't another precondition that BITS_PER_UNIT is 8 (because STRING_CSTs are
really bytes)?
> return check_ctz_string (ctor, val, zero_val, shiftval, input_bits);
>
> return false;
> @@ -1920,15 +1921,24 @@ simplify_count_trailing_zeroes (gimple_stmt_iterator
> *gsi)
> res_ops[1], res_ops[2], zero_val))
> {
> tree type = TREE_TYPE (res_ops[0]);
> - HOST_WIDE_INT ctzval;
> + HOST_WIDE_INT ctz_val = 0;
> HOST_WIDE_INT type_size = tree_to_shwi (TYPE_SIZE (type));
> - bool zero_ok = CTZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (type), ctzval) ==
> 2;
> + bool zero_ok =
> + CTZ_DEFINED_VALUE_AT_ZERO (SCALAR_INT_TYPE_MODE (type), ctz_val) == 2;
= shouldn't be at the end of line.
Jakub
