On Thu, Oct 26, 2017 at 11:48 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> The following testcase is miscompiled, because due to STV we end up
> with a load from V2DFmode MEM for which get_pool_entry returns
> a V1TImode vector. maybe_get_pool_constant doesn't have code
> to handle mode differences between what get_pool_constant returns
> and the requested mode. While it wouldn't be that hard to add,
> this all is done properly in avoid_constant_pool_reference already,
> which does everything maybe_get_pool_constant needs to do.
>
> So, this patch just removes the maybe_get_pool_constant function and
> uses avoid_constant_pool_reference instead. I've looked into history
> and the latter has been introduced in 2001, the former in 2002, but at
> that point the latter didn't do any delegitimization (but has the
> mode conversions at that point), while the former had hand-written
> delegitimization. But since then delegitimization has been added to
> a_c_p_r and custom delegitimization changed into the full blown one in
> m_g_p_c.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/7.x?
>
> 2017-10-26 Jakub Jelinek <ja...@redhat.com>
>
> PR target/82703
> * config/i386/i386-protos.h (maybe_get_pool_constant): Removed.
> * config/i386/i386.c (maybe_get_pool_constant): Removed.
> (ix86_split_to_parts): Use avoid_constant_pool_reference instead of
> maybe_get_pool_constant.
> * config/i386/predicates.md (zero_extended_scalar_load_operand):
> Likewise.
>
> * gcc.dg/pr82703.c: New test.
OK.
Thanks,
Uros.
> --- gcc/config/i386/i386-protos.h.jj 2017-04-20 12:19:54.000000000 +0200
> +++ gcc/config/i386/i386-protos.h 2017-10-26 13:14:05.172571175 +0200
> @@ -282,8 +282,6 @@ extern bool i386_pe_type_dllexport_p (tr
>
> extern int i386_pe_reloc_rw_mask (void);
>
> -extern rtx maybe_get_pool_constant (rtx);
> -
> extern char internal_label_prefix[16];
> extern int internal_label_prefix_len;
>
> --- gcc/config/i386/i386.c.jj 2017-09-30 10:26:43.000000000 +0200
> +++ gcc/config/i386/i386.c 2017-10-26 13:15:03.917891804 +0200
> @@ -19830,20 +19830,6 @@ ix86_expand_clear (rtx dest)
> emit_insn (tmp);
> }
>
> -/* X is an unchanging MEM. If it is a constant pool reference, return
> - the constant pool rtx, else NULL. */
> -
> -rtx
> -maybe_get_pool_constant (rtx x)
> -{
> - x = ix86_delegitimize_address (XEXP (x, 0));
> -
> - if (GET_CODE (x) == SYMBOL_REF && CONSTANT_POOL_ADDRESS_P (x))
> - return get_pool_constant (x);
> -
> - return NULL_RTX;
> -}
> -
> void
> ix86_expand_move (machine_mode mode, rtx operands[])
> {
> @@ -25371,11 +25357,7 @@ ix86_split_to_parts (rtx operand, rtx *p
> /* Optimize constant pool reference to immediates. This is used by fp
> moves, that force all constants to memory to allow combining. */
> if (MEM_P (operand) && MEM_READONLY_P (operand))
> - {
> - rtx tmp = maybe_get_pool_constant (operand);
> - if (tmp)
> - operand = tmp;
> - }
> + operand = avoid_constant_pool_reference (operand);
>
> if (MEM_P (operand) && !offsettable_memref_p (operand))
> {
> --- gcc/config/i386/predicates.md.jj 2017-04-20 12:19:54.000000000 +0200
> +++ gcc/config/i386/predicates.md 2017-10-26 13:16:27.399926361 +0200
> @@ -975,9 +975,9 @@ (define_predicate "zero_extended_scalar_
> (match_code "mem")
> {
> unsigned n_elts;
> - op = maybe_get_pool_constant (op);
> + op = avoid_constant_pool_reference (op);
>
> - if (!(op && GET_CODE (op) == CONST_VECTOR))
> + if (GET_CODE (op) != CONST_VECTOR)
> return false;
>
> n_elts = CONST_VECTOR_NUNITS (op);
> --- gcc/testsuite/gcc.dg/pr82703.c.jj 2017-10-26 13:19:23.879885830 +0200
> +++ gcc/testsuite/gcc.dg/pr82703.c 2017-10-26 13:18:42.000000000 +0200
> @@ -0,0 +1,28 @@
> +/* PR target/82703 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fno-tree-sra -ftree-vectorize" } */
> +
> +__attribute__((noinline, noclone)) void
> +compare (const double *p, const double *q)
> +{
> + for (int i = 0; i < 3; ++i)
> + if (p[i] != q[i])
> + __builtin_abort ();
> +}
> +
> +double vr[3] = { 4, 4, 4 };
> +
> +int
> +main ()
> +{
> + double v1[3] = { 1, 2, 3 };
> + double v2[3] = { 3, 2, 1 };
> + double v3[3];
> + __builtin_memcpy (v3, v1, sizeof (v1));
> + for (int i = 0; i < 3; ++i)
> + v3[i] += v2[i];
> + for (int i = 0; i < 3; ++i)
> + v1[i] += v2[i];
> + compare (v3, vr);
> + return 0;
> +}
>
> Jakub
