On Mon, 27 Jun 2016, Jakub Jelinek wrote:
> Hi!
>
> This patch is an attempt to fix ICE on the following testcase.
> In output_constant_pool_2 we assume that for vector modes, force_const_mem
> must be a CONST_VECTOR, but for the weirdo vector modes with single element
> it could very well be just a SUBREG of some other constant.
> This isn't enough though, e.g. for -fpic we shouldn't force it into constant
> pool constant, because it needs relocation.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-06-27 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/71626
> * output.h (compute_reloc_for_rtx): New prototype.
> * varasm.c (output_constant_pool_2): Handle SUBREGs for V1??mode
> vectors.
> (compute_reloc_for_rtx): No longer static.
> * config/i386/i386.c (ix86_expand_vector_move): For V1??mode SUBREG
> of scalar constant that needs relocation, force the constant into
> the inner mode reg first.
>
> * gcc.c-torture/execute/pr71626-1.c: New test.
> * gcc.c-torture/execute/pr71626-2.c: New test.
>
> --- gcc/output.h.jj 2016-01-04 14:55:53.000000000 +0100
> +++ gcc/output.h 2016-06-27 12:21:41.529484513 +0200
> @@ -349,6 +349,9 @@ extern bool decl_readonly_section (const
> given a constant expression. */
> extern int compute_reloc_for_constant (tree);
>
> +/* Similarly, but for RTX. */
> +extern int compute_reloc_for_rtx (const_rtx);
> +
> /* User label prefix in effect for this compilation. */
> extern const char *user_label_prefix;
>
> --- gcc/varasm.c.jj 2016-06-10 20:24:03.000000000 +0200
> +++ gcc/varasm.c 2016-06-27 12:21:36.062553957 +0200
> @@ -3834,6 +3834,17 @@ output_constant_pool_2 (machine_mode mod
> machine_mode submode = GET_MODE_INNER (mode);
> unsigned int subalign = MIN (align, GET_MODE_BITSIZE (submode));
>
> + /* For V1??mode, allow SUBREGs. */
> + if (GET_CODE (x) == SUBREG
> + && GET_MODE_NUNITS (mode) == 1
> + && GET_MODE_BITSIZE (submode) == GET_MODE_BITSIZE (mode)
> + && SUBREG_BYTE (x) == 0
> + && GET_MODE (SUBREG_REG (x)) == submode)
> + {
> + output_constant_pool_2 (submode, SUBREG_REG (x), align);
> + break;
> + }
> +
I wonder why we can rely on the MODE_FLOAT case not seeing
(subreg:DF (reg:V2DF ) 0) but have to handle
(subreg:V1DF (reg:DF ...) 0) in the MODE_VECTOR case.
I do remember trying to optimize constant pool handling to not
re-emit DFmode constants if we have a VxDFmode constant with
a matching component.
That said, if we don't allow arbitrary subregs maybe we should
adjust the constant_descriptor_rtx management code to assert
that only the above form of subregs can appear?
Richard.
> gcc_assert (GET_CODE (x) == CONST_VECTOR);
> units = CONST_VECTOR_NUNITS (x);
>
> @@ -6637,7 +6648,7 @@ compute_reloc_for_rtx_1 (const_rtx x)
> is a mask for which bit 1 indicates a global relocation, and bit 0
> indicates a local relocation. */
>
> -static int
> +int
> compute_reloc_for_rtx (const_rtx x)
> {
> switch (GET_CODE (x))
> --- gcc/config/i386/i386.c.jj 2016-06-24 12:59:29.000000000 +0200
> +++ gcc/config/i386/i386.c 2016-06-27 12:23:15.504290801 +0200
> @@ -19605,6 +19605,20 @@ ix86_expand_vector_move (machine_mode mo
> if (push_operand (op0, VOIDmode))
> op0 = emit_move_resolve_push (mode, op0);
>
> + /* For V1XXmode subregs of scalar constants, force the scalar into
> + reg first if it would need relocations. */
> + if (SUBREG_P (op1)
> + && CONSTANT_P (SUBREG_REG (op1))
> + && VECTOR_MODE_P (mode)
> + && GET_MODE_NUNITS (mode) == 1
> + && GET_MODE (SUBREG_REG (op1)) == GET_MODE_INNER (mode)
> + && compute_reloc_for_rtx (SUBREG_REG (op1)))
> + {
> + rtx r = force_reg (GET_MODE (SUBREG_REG (op1)), SUBREG_REG (op1));
> + op1 = simplify_gen_subreg (mode, r, GET_MODE (SUBREG_REG (op1)),
> + SUBREG_BYTE (op1));
> + }
> +
> /* Force constants other than zero into memory. We do not know how
> the instructions used to build constants modify the upper 64 bits
> of the register, once we have that information we may be able
> --- gcc/testsuite/gcc.c-torture/execute/pr71626-1.c.jj 2016-06-27
> 12:31:51.689755677 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr71626-1.c 2016-06-27
> 12:31:05.078344165 +0200
> @@ -0,0 +1,19 @@
> +/* PR middle-end/71626 */
> +
> +typedef __INTPTR_TYPE__ V __attribute__((__vector_size__(sizeof
> (__INTPTR_TYPE__))));
> +
> +__attribute__((noinline, noclone)) V
> +foo ()
> +{
> + V v = { (__INTPTR_TYPE__) foo };
> + return v;
> +}
> +
> +int
> +main ()
> +{
> + V v = foo ();
> + if (v[0] != (__INTPTR_TYPE__) foo)
> + __builtin_abort ();
> + return 0;
> +}
> --- gcc/testsuite/gcc.c-torture/execute/pr71626-2.c.jj 2016-06-27
> 12:31:54.837715933 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr71626-2.c 2016-06-27
> 12:31:47.977802542 +0200
> @@ -0,0 +1,4 @@
> +/* PR middle-end/71626 */
> +/* { dg-additional-options "-fpic" { target fpic } } */
> +
> +#include "pr71626-1.c"
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
