On Wed, 10 Apr 2019, Jakub Jelinek wrote:
> Hi!
>
> In r268957 aka PR66152 fix I've slightly changed the store_expr
> code which does store_by_pieces for just a part of an array and
> clear_storage for the rest and since then the following testcase
> is miscompiled on s390x-linux. I believe one could construct other testcase
> that would be miscompiled before as well though.
>
> The problem is that store_by_pieces with RETURN_END returns a MEM object
> with QImode and thus also MEM_SIZE of 1, we then adjust_address it to
> BLKmode which doesn't change MEM_SIZE and nothing at least in the generic
> clear_storage code tweaks that MEM_SIZE later on (and while perhaps some
> targets do, e.g. s390 does not). Because in this case the size is
> really known and constant, the following patch makes sure we do
> what adjust_address does, but in addition make sure to set MEM_SIZE
> properly. When MEM_SIZE is 1 and in the testcase there is a store after
> it to the first few bytes of the remainder, RTL DSE happily removes that
> memory clear insn emitted for clear_storage, because if it was really size
> 1, it would be indeed redundant, but there are 20 bytes that are not
> redundant after it.
>
> Bootstrapped/regtested on {x86_64,i686,s390x,powerpc64le}-linux, ok for
> trunk?
OK.
Richard.
> 2019-04-10 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/90025
> * expr.c (store_expr): Set properly size on the MEM passed to
> clear_storage.
>
> * gcc.c-torture/execute/pr90025.c: New test.
>
> --- gcc/expr.c.jj 2019-02-21 00:01:59.418057433 +0100
> +++ gcc/expr.c 2019-04-09 12:49:20.704457867 +0200
> @@ -5658,7 +5658,8 @@ store_expr (tree exp, rtx target, int ca
> dest_mem = store_by_pieces (target, str_copy_len, string_cst_read_str,
> (void *) str, MEM_ALIGN (target), false,
> RETURN_END);
> - clear_storage (adjust_address (dest_mem, BLKmode, 0),
> + clear_storage (adjust_address_1 (dest_mem, BLKmode, 0, 1, 1, 0,
> + exp_len - str_copy_len),
> GEN_INT (exp_len - str_copy_len), BLOCK_OP_NORMAL);
> return NULL_RTX;
> }
> --- gcc/testsuite/gcc.c-torture/execute/pr90025.c.jj 2019-04-09
> 12:59:35.876449686 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr90025.c 2019-04-09
> 12:59:20.156705430 +0200
> @@ -0,0 +1,28 @@
> +/* PR middle-end/90025 */
> +
> +__attribute__((noipa)) void
> +bar (char *p)
> +{
> + int i;
> + for (i = 0; i < 6; i++)
> + if (p[i] != "foobar"[i])
> + __builtin_abort ();
> + for (; i < 32; i++)
> + if (p[i] != '\0')
> + __builtin_abort ();
> +}
> +
> +__attribute__((noipa)) void
> +foo (unsigned int x)
> +{
> + char s[32] = { 'f', 'o', 'o', 'b', 'a', 'r', 0 };
> + ((unsigned int *) s)[2] = __builtin_bswap32 (x);
> + bar (s);
> +}
> +
> +int
> +main ()
> +{
> + foo (0);
> + return 0;
> +}
>
> Jakub
>
--
Richard Biener <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
