diff mbox

Fix ICE with V1DImode ctor (PR middle-end/71626)

Message ID 20160627181618.GQ7387@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek June 27, 2016, 6:16 p.m. UTC
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  <jakub@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.


	Jakub

Comments

Richard Biener June 28, 2016, 7:15 a.m. UTC | #1
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  <jakub@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
> 
>
diff mbox

Patch

--- 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;
+	  }
+
 	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"