[PATCH] Fix ICE during MEM_REF expansion (PR middle-end/90840)

[PATCH] Fix ICE during MEM_REF expansion (PR middle-end/90840)

[Jakub Jelinek]

Hi!

On the following testcase we ICE on i686-linux (32-bit), because we store
(first 96-bit, then 72-bit) structure into the first part of a 2x 96-bit
complex long double, and for 96-bit floats there is no corresponding
integral mode that covers it and we ICE when op0 is not in MEM (it is a
REG).
The following patch handles the simple case where the whole dest REG is
covered and value is a MEM using a load from the memory, and for the rest
just spills on the stack, similarly how we punt when for stores to complex
REGs if the bitsize/bitnum cover portions of both halves.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-11-19  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/90840
	* expmed.c (store_bit_field_1): Handle the case where op0 is not a MEM
	and has a mode that doesn't have corresponding integral type.

	* gcc.c-torture/compile/pr90840.c: New test.

--- gcc/expmed.c.jj	2019-11-15 00:37:32.000000000 +0100
+++ gcc/expmed.c	2019-11-19 17:09:22.035129617 +0100
@@ -840,6 +840,27 @@ store_bit_field_1 (rtx str_rtx, poly_uin
       if (MEM_P (op0))
 	op0 = adjust_bitfield_address_size (op0, op0_mode.else_blk (),
 					    0, MEM_SIZE (op0));
+      else if (!op0_mode.exists ())
+	{
+	  if (ibitnum == 0
+	      && known_eq (ibitsize, GET_MODE_BITSIZE (GET_MODE (op0)))
+	      && MEM_P (value)
+	      && !reverse)
+	    {
+	      value = adjust_address (value, GET_MODE (op0), 0);
+	      emit_move_insn (op0, value);
+	      return true;
+	    }
+	  if (!fallback_p)
+	    return false;
+	  rtx temp = assign_stack_temp (GET_MODE (op0),
+					GET_MODE_SIZE (GET_MODE (op0)));
+	  emit_move_insn (temp, op0);
+	  store_bit_field_1 (temp, bitsize, bitnum, 0, 0, fieldmode, value,
+			     reverse, fallback_p);
+	  emit_move_insn (op0, temp);
+	  return true;
+	}
       else
 	op0 = gen_lowpart (op0_mode.require (), op0);
     }
--- gcc/testsuite/gcc.c-torture/compile/pr90840.c.jj	2019-11-19 17:18:31.361918896 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr90840.c	2019-11-19 17:11:06.010575339 +0100
@@ -0,0 +1,19 @@
+/* PR middle-end/90840 */
+struct S { long long a; int b; };
+struct S foo (void);
+struct __attribute__((packed)) T { long long a; char b; };
+struct T baz (void);
+
+void
+bar (void)
+{
+  _Complex long double c;
+  *(struct S *) &c = foo ();
+}
+
+void
+qux (void)
+{
+  _Complex long double c;
+  *(struct T *) &c = baz ();
+}

	Jakub

Re: [PATCH] Fix ICE during MEM_REF expansion (PR middle-end/90840)

[Jeff Law]

On 11/19/19 4:42 PM, Jakub Jelinek wrote:
> Hi!
> 
> On the following testcase we ICE on i686-linux (32-bit), because we store
> (first 96-bit, then 72-bit) structure into the first part of a 2x 96-bit
> complex long double, and for 96-bit floats there is no corresponding
> integral mode that covers it and we ICE when op0 is not in MEM (it is a
> REG).
> The following patch handles the simple case where the whole dest REG is
> covered and value is a MEM using a load from the memory, and for the rest
> just spills on the stack, similarly how we punt when for stores to complex
> REGs if the bitsize/bitnum cover portions of both halves.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-11-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/90840
> 	* expmed.c (store_bit_field_1): Handle the case where op0 is not a MEM
> 	and has a mode that doesn't have corresponding integral type.
> 
> 	* gcc.c-torture/compile/pr90840.c: New test.
OK
jeff

Re: [PATCH] Fix ICE during MEM_REF expansion (PR middle-end/90840)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 2796 bytes --]

On Wed, 20 Nov 2019, Jakub Jelinek wrote:

> Hi!
> 
> On the following testcase we ICE on i686-linux (32-bit), because we store
> (first 96-bit, then 72-bit) structure into the first part of a 2x 96-bit
> complex long double, and for 96-bit floats there is no corresponding
> integral mode that covers it and we ICE when op0 is not in MEM (it is a
> REG).
> The following patch handles the simple case where the whole dest REG is
> covered and value is a MEM using a load from the memory, and for the rest
> just spills on the stack, similarly how we punt when for stores to complex
> REGs if the bitsize/bitnum cover portions of both halves.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-11-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/90840
> 	* expmed.c (store_bit_field_1): Handle the case where op0 is not a MEM
> 	and has a mode that doesn't have corresponding integral type.
> 
> 	* gcc.c-torture/compile/pr90840.c: New test.
> 
> --- gcc/expmed.c.jj	2019-11-15 00:37:32.000000000 +0100
> +++ gcc/expmed.c	2019-11-19 17:09:22.035129617 +0100
> @@ -840,6 +840,27 @@ store_bit_field_1 (rtx str_rtx, poly_uin
>        if (MEM_P (op0))
>  	op0 = adjust_bitfield_address_size (op0, op0_mode.else_blk (),
>  					    0, MEM_SIZE (op0));
> +      else if (!op0_mode.exists ())
> +	{
> +	  if (ibitnum == 0
> +	      && known_eq (ibitsize, GET_MODE_BITSIZE (GET_MODE (op0)))
> +	      && MEM_P (value)
> +	      && !reverse)
> +	    {
> +	      value = adjust_address (value, GET_MODE (op0), 0);
> +	      emit_move_insn (op0, value);
> +	      return true;
> +	    }
> +	  if (!fallback_p)
> +	    return false;
> +	  rtx temp = assign_stack_temp (GET_MODE (op0),
> +					GET_MODE_SIZE (GET_MODE (op0)));
> +	  emit_move_insn (temp, op0);
> +	  store_bit_field_1 (temp, bitsize, bitnum, 0, 0, fieldmode, value,
> +			     reverse, fallback_p);
> +	  emit_move_insn (op0, temp);
> +	  return true;
> +	}
>        else
>  	op0 = gen_lowpart (op0_mode.require (), op0);
>      }
> --- gcc/testsuite/gcc.c-torture/compile/pr90840.c.jj	2019-11-19 17:18:31.361918896 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr90840.c	2019-11-19 17:11:06.010575339 +0100
> @@ -0,0 +1,19 @@
> +/* PR middle-end/90840 */
> +struct S { long long a; int b; };
> +struct S foo (void);
> +struct __attribute__((packed)) T { long long a; char b; };
> +struct T baz (void);
> +
> +void
> +bar (void)
> +{
> +  _Complex long double c;
> +  *(struct S *) &c = foo ();
> +}
> +
> +void
> +qux (void)
> +{
> +  _Complex long double c;
> +  *(struct T *) &c = baz ();
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH] Fix ICE during MEM_REF expansion (PR middle-end/90840)

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

> On the following testcase we ICE on i686-linux (32-bit), because we store
> (first 96-bit, then 72-bit) structure into the first part of a 2x 96-bit
> complex long double, and for 96-bit floats there is no corresponding
> integral mode that covers it and we ICE when op0 is not in MEM (it is a
> REG).

The test triggers an ICE in simplify_subreg because the innermode is BLKmode:

  gcc_assert (innermode != BLKmode);

on PowerPC64/VxWorks at -O1 and above.  This comes from this call:

		  if (MEM_P (result))
 		    from_rtx = change_address (result, to_mode, NULL_RTX);
 		  else
 		    from_rtx
		      = simplify_gen_subreg (to_mode, result,
					     TYPE_MODE (TREE_TYPE (from)), 0);

in expand_assignment, where 'result' is not a MEM despite TREE_TYPE (from) 
having BLKmode.  That's as expected because 'from' is a CALL_EXPR whose result 
is returned as a TImode register to avoid using BLKmode and thus spilling it 
to memory in between.

It turns out that simplify_subreg contains another assertion:

  gcc_assert (GET_MODE (op) == innermode
	      || GET_MODE (op) == VOIDmode);

so we can equivalently pass GET_MODE (result) as the mode, except when it is 
VOIDmode in which case we can still use TYPE_MODE (TREE_TYPE (from)).

Tested on PowerPC64/VxWorks and x86-64/Linux, applied on mainline as obvious.


2019-12-07  Eric Botcazou  <ebotcazou@adacore.com>

	PR middle-end/90840
	* expr.c (expand_assignment): In the case of a CONCAT on the LHS, make
	sure to pass a valid inner mode in calls to simplify_gen_subreg.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1298 bytes --]

Index: expr.c
===================================================================
--- expr.c	(revision 278938)
+++ expr.c	(working copy)
@@ -5285,13 +5285,16 @@ expand_assignment (tree to, tree from, b
 		}
 	      else
 		{
+		  machine_mode from_mode
+		    = GET_MODE (result) == VOIDmode
+		      ? TYPE_MODE (TREE_TYPE (from))
+		      : GET_MODE (result);
 		  rtx from_rtx;
 		  if (MEM_P (result))
 		    from_rtx = change_address (result, to_mode, NULL_RTX);
 		  else
 		    from_rtx
-		      = simplify_gen_subreg (to_mode, result,
-					     TYPE_MODE (TREE_TYPE (from)), 0);
+		      = simplify_gen_subreg (to_mode, result, from_mode, 0);
 		  if (from_rtx)
 		    {
 		      emit_move_insn (XEXP (to_rtx, 0),
@@ -5303,12 +5306,9 @@ expand_assignment (tree to, tree from, b
 		    {
 		      to_mode = GET_MODE_INNER (to_mode);
 		      rtx from_real
-			= simplify_gen_subreg (to_mode, result,
-					       TYPE_MODE (TREE_TYPE (from)),
-					       0);
+			= simplify_gen_subreg (to_mode, result, from_mode, 0);
 		      rtx from_imag
-			= simplify_gen_subreg (to_mode, result,
-					       TYPE_MODE (TREE_TYPE (from)),
+			= simplify_gen_subreg (to_mode, result, from_mode,
 					       GET_MODE_SIZE (to_mode));
 		      if (!from_real || !from_imag)
 			goto concat_store_slow;