On Thu, 5 Oct 2017, Jakub Jelinek wrote:
> Hi!
>
> The following testcase fails, because can_native_encode_type_p doesn't
> handle BOOLEAN_TYPE nor ENUMERAL_TYPE (while native_encode_expr handles
> those just fine).
> But, it isn't just those, can_native_encode_type_p doesn't really make
> sense to me, since whether native_encode_expr fails or not doesn't
> really depend on the expression type, but rather on what exact tcc_constant
> the expression is, what size it has and some other properties of the
> expression.
> Instead of writing a routine similar to can_native_encode_string_p that
> would handle all the cases when native_encode_expr fails, I've changed
> native_encode_expr itself, so that it has a faster dry run mode, where
> ptr is NULL, which doesn't store anything, but just returns what it would
> return given a non-NULL ptr.
> The patch then changes vectorizable_store as well as store merging to use
> this to check whether native_encode_expr will be successful.
>
> In addition to that, I've found a thinko in store merging stmt counting,
> where it would unnecessarily look for 3rd non-debug stmt even when 2
> stmts is what it checks after the loop. And store merging was for some
> unknown reason calling native_encode_expr with 4 arguments, while the
> standard/preferred way to call it is with 3 arguments, then it verifies
> whether the constant encoding can fit into the buffer etc.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Thanks,
Richard.
> 2017-10-05 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/82434
> * fold-const.h (can_native_encode_type_p,
> can_native_encode_string_p): Remove.
> * fold-const.c (native_encode_int): Formatting fixes. If ptr is NULL,
> don't encode anything, just return what would be otherwise returned.
> (native_encode_fixed, native_encode_complex, native_encode_vector):
> Likewise.
> (native_encode_string): Likewise. Inline by hand
> can_native_encode_string_p.
> (can_native_encode_type_p): Remove.
> (can_native_encode_string_p): Remove.
> * tree-vect-stmts.c (vectorizable_store): Instead of testing just
> STRING_CSTs using can_native_encode_string_p, test all
> CONSTANT_CLASS_P values using native_encode_expr with NULL ptr.
> * gimple-ssa-store-merging.c (encode_tree_to_bitpos): Remove last
> argument from native_encode_expr.
> (rhs_valid_for_store_merging_p): Use native_encode_expr with NULL ptr.
> (pass_store_merging::execute): Don't unnecessarily look for 3 stmts,
> but just 2.
>
> * gcc.dg/store_merging_9.c: New test.
>
> --- gcc/fold-const.h.jj 2017-09-05 23:28:10.000000000 +0200
> +++ gcc/fold-const.h 2017-10-05 13:16:27.355770215 +0200
> @@ -27,8 +27,6 @@ extern int folding_initializer;
> /* Convert between trees and native memory representation. */
> extern int native_encode_expr (const_tree, unsigned char *, int, int off =
> -1);
> extern tree native_interpret_expr (tree, const unsigned char *, int);
> -extern bool can_native_encode_type_p (tree);
> -extern bool can_native_encode_string_p (const_tree);
>
> /* Fold constants as much as possible in an expression.
> Returns the simplified expression.
> --- gcc/fold-const.c.jj 2017-10-04 16:45:28.000000000 +0200
> +++ gcc/fold-const.c 2017-10-05 13:17:42.195863063 +0200
> @@ -6982,11 +6982,15 @@ native_encode_int (const_tree expr, unsi
> int byte, offset, word, words;
> unsigned char value;
>
> - if ((off == -1 && total_bytes > len)
> - || off >= total_bytes)
> + if ((off == -1 && total_bytes > len) || off >= total_bytes)
> return 0;
> if (off == -1)
> off = 0;
> +
> + if (ptr == NULL)
> + /* Dry run. */
> + return MIN (len, total_bytes - off);
> +
> words = total_bytes / UNITS_PER_WORD;
>
> for (byte = 0; byte < total_bytes; byte++)
> @@ -7009,8 +7013,7 @@ native_encode_int (const_tree expr, unsi
> }
> else
> offset = BYTES_BIG_ENDIAN ? (total_bytes - 1) - byte : byte;
> - if (offset >= off
> - && offset - off < len)
> + if (offset >= off && offset - off < len)
> ptr[offset - off] = value;
> }
> return MIN (len, total_bytes - off);
> @@ -7036,8 +7039,7 @@ native_encode_fixed (const_tree expr, un
>
> i_type = lang_hooks.types.type_for_size (GET_MODE_BITSIZE (mode), 1);
>
> - if (NULL_TREE == i_type
> - || TYPE_PRECISION (i_type) != total_bytes)
> + if (NULL_TREE == i_type || TYPE_PRECISION (i_type) != total_bytes)
> return 0;
>
> value = TREE_FIXED_CST (expr);
> @@ -7065,11 +7067,15 @@ native_encode_real (const_tree expr, uns
> up to 192 bits. */
> long tmp[6];
>
> - if ((off == -1 && total_bytes > len)
> - || off >= total_bytes)
> + if ((off == -1 && total_bytes > len) || off >= total_bytes)
> return 0;
> if (off == -1)
> off = 0;
> +
> + if (ptr == NULL)
> + /* Dry run. */
> + return MIN (len, total_bytes - off);
> +
> words = (32 / BITS_PER_UNIT) / UNITS_PER_WORD;
>
> real_to_target (tmp, TREE_REAL_CST_PTR (expr), TYPE_MODE (type));
> @@ -7123,15 +7129,14 @@ native_encode_complex (const_tree expr,
>
> part = TREE_REALPART (expr);
> rsize = native_encode_expr (part, ptr, len, off);
> - if (off == -1
> - && rsize == 0)
> + if (off == -1 && rsize == 0)
> return 0;
> part = TREE_IMAGPART (expr);
> if (off != -1)
> off = MAX (0, off - GET_MODE_SIZE (SCALAR_TYPE_MODE (TREE_TYPE (part))));
> - isize = native_encode_expr (part, ptr+rsize, len-rsize, off);
> - if (off == -1
> - && isize != rsize)
> + isize = native_encode_expr (part, ptr ? ptr + rsize : NULL,
> + len - rsize, off);
> + if (off == -1 && isize != rsize)
> return 0;
> return rsize + isize;
> }
> @@ -7161,9 +7166,9 @@ native_encode_vector (const_tree expr, u
> continue;
> }
> elem = VECTOR_CST_ELT (expr, i);
> - int res = native_encode_expr (elem, ptr+offset, len-offset, off);
> - if ((off == -1 && res != size)
> - || res == 0)
> + int res = native_encode_expr (elem, ptr ? ptr + offset : NULL,
> + len - offset, off);
> + if ((off == -1 && res != size) || res == 0)
> return 0;
> offset += res;
> if (offset >= len)
> @@ -7183,16 +7188,24 @@ native_encode_vector (const_tree expr, u
> static int
> native_encode_string (const_tree expr, unsigned char *ptr, int len, int off)
> {
> - if (! can_native_encode_string_p (expr))
> + tree type = TREE_TYPE (expr);
> +
> + /* Wide-char strings are encoded in target byte-order so native
> + encoding them is trivial. */
> + if (BITS_PER_UNIT != CHAR_BIT
> + || TREE_CODE (type) != ARRAY_TYPE
> + || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
> + || !tree_fits_shwi_p (TYPE_SIZE_UNIT (type)))
> return 0;
>
> HOST_WIDE_INT total_bytes = tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE
> (expr)));
> - if ((off == -1 && total_bytes > len)
> - || off >= total_bytes)
> + if ((off == -1 && total_bytes > len) || off >= total_bytes)
> return 0;
> if (off == -1)
> off = 0;
> - if (TREE_STRING_LENGTH (expr) - off < MIN (total_bytes, len))
> + if (ptr == NULL)
> + /* Dry run. */;
> + else if (TREE_STRING_LENGTH (expr) - off < MIN (total_bytes, len))
> {
> int written = 0;
> if (off < TREE_STRING_LENGTH (expr))
> @@ -7211,7 +7224,8 @@ native_encode_string (const_tree expr, u
>
> /* Subroutine of fold_view_convert_expr. Encode the INTEGER_CST,
> REAL_CST, COMPLEX_CST or VECTOR_CST specified by EXPR into the
> - buffer PTR of length LEN bytes. If OFF is not -1 then start
> + buffer PTR of length LEN bytes. If PTR is NULL, don't actually store
> + anything, just do a dry run. If OFF is not -1 then start
> the encoding at byte offset OFF and encode at most LEN bytes.
> Return the number of bytes placed in the buffer, or zero upon failure. */
>
> @@ -7459,43 +7473,6 @@ can_native_interpret_type_p (tree type)
> }
> }
>
> -/* Return true iff a constant of type TYPE is accepted by
> - native_encode_expr. */
> -
> -bool
> -can_native_encode_type_p (tree type)
> -{
> - switch (TREE_CODE (type))
> - {
> - case INTEGER_TYPE:
> - case REAL_TYPE:
> - case FIXED_POINT_TYPE:
> - case COMPLEX_TYPE:
> - case VECTOR_TYPE:
> - case POINTER_TYPE:
> - return true;
> - default:
> - return false;
> - }
> -}
> -
> -/* Return true iff a STRING_CST S is accepted by
> - native_encode_expr. */
> -
> -bool
> -can_native_encode_string_p (const_tree expr)
> -{
> - tree type = TREE_TYPE (expr);
> -
> - /* Wide-char strings are encoded in target byte-order so native
> - encoding them is trivial. */
> - if (BITS_PER_UNIT != CHAR_BIT
> - || TREE_CODE (type) != ARRAY_TYPE
> - || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
> - || !tree_fits_shwi_p (TYPE_SIZE_UNIT (type)))
> - return false;
> - return true;
> -}
>
> /* Fold a VIEW_CONVERT_EXPR of a constant expression EXPR to type
> TYPE at compile-time. If we're unable to perform the conversion
> --- gcc/tree-vect-stmts.c.jj 2017-09-22 20:51:51.000000000 +0200
> +++ gcc/tree-vect-stmts.c 2017-10-05 13:26:06.609750959 +0200
> @@ -5728,10 +5728,9 @@ vectorizable_store (gimple *stmt, gimple
>
> op = gimple_assign_rhs1 (stmt);
>
> - /* In the case this is a store from a STRING_CST make sure
> + /* In the case this is a store from a constant make sure
> native_encode_expr can handle it. */
> - if (TREE_CODE (op) == STRING_CST
> - && ! can_native_encode_string_p (op))
> + if (CONSTANT_CLASS_P (op) && native_encode_expr (op, NULL, 64) == 0)
> return false;
>
> if (!vect_is_simple_use (op, vinfo, &def_stmt, &dt, &rhs_vectype))
> --- gcc/gimple-ssa-store-merging.c.jj 2017-09-13 16:22:25.000000000 +0200
> +++ gcc/gimple-ssa-store-merging.c 2017-10-05 13:40:55.792985363 +0200
> @@ -357,8 +357,7 @@ encode_tree_to_bitpos (tree expr, unsign
> || !int_mode_for_size (bitlen, 0).exists ());
>
> if (!sub_byte_op_p)
> - return (native_encode_expr (tmp_int, ptr + first_byte, total_bytes, 0)
> - != 0);
> + return native_encode_expr (tmp_int, ptr + first_byte, total_bytes) != 0;
>
> /* LITTLE-ENDIAN
> We are writing a non byte-sized quantity or at a position that is not
> @@ -408,7 +407,7 @@ encode_tree_to_bitpos (tree expr, unsign
> memset (tmpbuf, '\0', byte_size);
> /* The store detection code should only have allowed constants that are
> accepted by native_encode_expr. */
> - if (native_encode_expr (expr, tmpbuf, byte_size - 1, 0) == 0)
> + if (native_encode_expr (expr, tmpbuf, byte_size - 1) == 0)
> gcc_unreachable ();
>
> /* The native_encode_expr machinery uses TYPE_MODE to determine how many
> @@ -1326,12 +1325,8 @@ lhs_valid_for_store_merging_p (tree lhs)
> static bool
> rhs_valid_for_store_merging_p (tree rhs)
> {
> - tree type = TREE_TYPE (rhs);
> - if (TREE_CODE_CLASS (TREE_CODE (rhs)) != tcc_constant
> - || !can_native_encode_type_p (type))
> - return false;
> -
> - return true;
> + return native_encode_expr (rhs, NULL,
> + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (rhs)))) != 0;
> }
>
> /* Entry point for the pass. Go over each basic block recording chains of
> @@ -1357,7 +1352,7 @@ pass_store_merging::execute (function *f
> if (is_gimple_debug (gsi_stmt (gsi)))
> continue;
>
> - if (++num_statements > 2)
> + if (++num_statements >= 2)
> break;
> }
>
> --- gcc/testsuite/gcc.dg/store_merging_9.c.jj 2017-10-05 13:33:42.248234414
> +0200
> +++ gcc/testsuite/gcc.dg/store_merging_9.c 2017-10-05 13:39:09.162276372
> +0200
> @@ -0,0 +1,33 @@
> +/* PR tree-optimization/82434 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target store_merge } */
> +/* { dg-options "-O2 -fdump-tree-store-merging" } */
> +
> +enum E { E0, E1, E2 = __INT_MAX__, E3 = -__INT_MAX__ - 1 };
> +
> +struct bar {
> + enum E a;
> + char b;
> + _Bool c;
> + short d;
> +};
> +
> +void
> +foo1 (struct bar *p)
> +{
> + p->b = 0;
> + p->a = E0;
> + p->c = (_Bool) 0;
> + p->d = 0;
> +}
> +
> +void
> +foo2 (struct bar *p)
> +{
> + p->b = 0;
> + p->a = E0;
> + p->c = (_Bool) 1;
> + p->d = 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Merging successful" 2 "store-merging"
> } } */
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
