Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Eric Botcazou <ebotcazou@adacore.com>]

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

> I have noticed new failures after this commit (r244249).
> g++.dg/opt/call3.C fails at execution on armeb targets
> g++.dg/opt/call2.C fails at execution on aarch64_be

It turns out that there is already a big-endian adjustment a few lines above:

      /* If the value has a record type and an integral mode then, if BITSIZE
	 is narrower than this mode and this is for big-endian data, we must
        first put the value into the low-order bits.  Moreover, the field may
	 be not aligned on a byte boundary; in this case, if it has reverse
	storage order, it needs to be accessed as a scalar field with reverse
	 storage order and we must first put the value into target order.  */
      if (TREE_CODE (TREE_TYPE (exp)) == RECORD_TYPE
	  && GET_MODE_CLASS (GET_MODE (temp)) == MODE_INT)
	{
	  HOST_WIDE_INT size = GET_MODE_BITSIZE (GET_MODE (temp));

	  reverse = TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (exp));

	  if (reverse)
	    temp = flip_storage_order (GET_MODE (temp), temp);

	  if (bitsize < size
	      && reverse ? !BYTES_BIG_ENDIAN : BYTES_BIG_ENDIAN)
	    temp = expand_shift (RSHIFT_EXPR, GET_MODE (temp), temp,
				 size - bitsize, NULL_RTX, 1);
	}

and adding the extraction leads to a double adjustment on armeb, whereas no 
adjustment is still applied to aarch64_be...

That's why the attached patch merges the second adjustment in the first one by 
moving up the code fetching the return value from the registers; this ensures 
that only one adjustment is ever applied and that it is applied only on left- 
justified values.  But a final extraction (without big-endian adjustment) is 
still needed for small BLKmode values, otherwise store_bit_field aborts...

Tested on the same platforms as the original patch, applied as obvious.


        * expr.c (store_field): In the bitfield case, fetch the return value
	from the registers before applying a single big-endian adjustment.
	Always do a final load for a BLKmode value not larger than a word.

-- 
Eric Botcazou

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

Index: expr.c
===================================================================
--- expr.c	(revision 244258)
+++ expr.c	(working copy)
@@ -6832,13 +6832,36 @@ store_field (rtx target, HOST_WIDE_INT b
 
       temp = expand_normal (exp);
 
-      /* If the value has a record type and an integral mode then, if BITSIZE
-	 is narrower than this mode and this is for big-endian data, we must
-	 first put the value into the low-order bits.  Moreover, the field may
-	 be not aligned on a byte boundary; in this case, if it has reverse
-	 storage order, it needs to be accessed as a scalar field with reverse
-	 storage order and we must first put the value into target order.  */
-      if (TREE_CODE (TREE_TYPE (exp)) == RECORD_TYPE
+      /* Handle calls that return values in multiple non-contiguous locations.
+	 The Irix 6 ABI has examples of this.  */
+      if (GET_CODE (temp) == PARALLEL)
+	{
+	  HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp));
+	  machine_mode temp_mode
+	    = smallest_mode_for_size (size * BITS_PER_UNIT, MODE_INT);
+	  rtx temp_target = gen_reg_rtx (temp_mode);
+	  emit_group_store (temp_target, temp, TREE_TYPE (exp), size);
+	  temp = temp_target;
+	}
+
+      /* Handle calls that return BLKmode values in registers.  */
+      else if (mode == BLKmode && REG_P (temp) && TREE_CODE (exp) == CALL_EXPR)
+	{
+	  rtx temp_target = gen_reg_rtx (GET_MODE (temp));
+	  copy_blkmode_from_reg (temp_target, temp, TREE_TYPE (exp));
+	  temp = temp_target;
+	}
+
+      /* If the value has aggregate type and an integral mode then, if BITSIZE
+	 is narrower than this mode and this is for big-endian data, we first
+	 need to put the value into the low-order bits for store_bit_field,
+	 except when MODE is BLKmode and BITSIZE larger than the word size
+	 (see the handling of fields larger than a word in store_bit_field).
+	 Moreover, the field may be not aligned on a byte boundary; in this
+	 case, if it has reverse storage order, it needs to be accessed as a
+	 scalar field with reverse storage order and we must first put the
+	 value into target order.  */
+      if (AGGREGATE_TYPE_P (TREE_TYPE (exp))
 	  && GET_MODE_CLASS (GET_MODE (temp)) == MODE_INT)
 	{
 	  HOST_WIDE_INT size = GET_MODE_BITSIZE (GET_MODE (temp));
@@ -6849,7 +6872,8 @@ store_field (rtx target, HOST_WIDE_INT b
 	    temp = flip_storage_order (GET_MODE (temp), temp);
 
 	  if (bitsize < size
-	      && reverse ? !BYTES_BIG_ENDIAN : BYTES_BIG_ENDIAN)
+	      && reverse ? !BYTES_BIG_ENDIAN : BYTES_BIG_ENDIAN
+	      && !(mode == BLKmode && bitsize > BITS_PER_WORD))
 	    temp = expand_shift (RSHIFT_EXPR, GET_MODE (temp), temp,
 				 size - bitsize, NULL_RTX, 1);
 	}
@@ -6859,12 +6883,10 @@ store_field (rtx target, HOST_WIDE_INT b
 	  && mode != TYPE_MODE (TREE_TYPE (exp)))
 	temp = convert_modes (mode, TYPE_MODE (TREE_TYPE (exp)), temp, 1);
 
-      /* If TEMP is not a PARALLEL (see below) and its mode and that of TARGET
-	 are both BLKmode, both must be in memory and BITPOS must be aligned
-	 on a byte boundary.  If so, we simply do a block copy.  Likewise for
-	 a BLKmode-like TARGET.  */
-      if (GET_CODE (temp) != PARALLEL
-	  && GET_MODE (temp) == BLKmode
+      /* If the mode of TEMP and TARGET is BLKmode, both must be in memory
+	 and BITPOS must be aligned on a byte boundary.  If so, we simply do
+	 a block copy.  Likewise for a BLKmode-like TARGET.  */
+      if (GET_MODE (temp) == BLKmode
 	  && (GET_MODE (target) == BLKmode
 	      || (MEM_P (target)
 		  && GET_MODE_CLASS (GET_MODE (target)) == MODE_INT
@@ -6883,31 +6905,9 @@ store_field (rtx target, HOST_WIDE_INT b
 	  return const0_rtx;
 	}
 
-      /* Handle calls that return values in multiple non-contiguous locations.
-	 The Irix 6 ABI has examples of this.  */
-      if (GET_CODE (temp) == PARALLEL)
-	{
-	  HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp));
-	  machine_mode temp_mode
-	    = smallest_mode_for_size (size * BITS_PER_UNIT, MODE_INT);
-	  rtx temp_target = gen_reg_rtx (temp_mode);
-	  emit_group_store (temp_target, temp, TREE_TYPE (exp), size);
-	  temp = temp_target;
-	}
-
-      /* Handle calls that return BLKmode values in registers.  */
-      else if (mode == BLKmode && REG_P (temp) && TREE_CODE (exp) == CALL_EXPR)
-	{
-	  rtx temp_target = gen_reg_rtx (GET_MODE (temp));
-	  copy_blkmode_from_reg (temp_target, temp, TREE_TYPE (exp));
-	  temp = temp_target;
-	}
-
-      /* The behavior of store_bit_field is awkward when mode is BLKmode:
-	 it always takes its value from the lsb up to the word size but
-	 expects it left justified beyond it.  At this point TEMP is left
-	 justified so extract the value in the former case.  */
-      if (mode == BLKmode && bitsize <= BITS_PER_WORD)
+      /* If the mode of TEMP is still BLKmode and BITSIZE not larger than the
+	 word size, we need to load the value (see again store_bit_field).  */
+      if (GET_MODE (temp) == BLKmode && bitsize <= BITS_PER_WORD)
 	{
 	  machine_mode temp_mode = smallest_mode_for_size (bitsize, MODE_INT);
 	  temp = extract_bit_field (temp, bitsize, 0, 1, NULL_RTX, temp_mode,