Re: [PATCH] expand: Improve tuple-extraction handling.

> Understood and understandable.  We could also just spill directly (so 
> the insertion/extraction risk is averted) in operand_subword_force and 
> move_multi_word for the problematic cases?  This still feels ad-hoc 
> but maybe not at the wrong layer at least.

If I didn't mess up anything during rebasing and testing, the attached 
appears to work for aarch64 and riscv with the exception (as before) of 
concatn in lower-subreg.  Some comments and the changelog are still out 
of date... but the spill change in operand_subword force seems to be 
sufficient, no multi_word change necessary.
Another store_bit_field hunk I still added, though.  That one looks in 
line with the others we have.

Regards
 Robin

	PR target/124996

gcc/ChangeLog:

	* config/aarch64/aarch64.cc (aarch64_regmode_natural_size):
	Return full vectors for all vector modes.
	* config/riscv/riscv-v.cc (shuffle_even_odd_patterns): Remove
	workaround.
	* config/riscv/riscv.cc (riscv_regmode_natural_size): Return
	full vector size.
	* expmed.cc (store_bit_field_1): Spill unsplittable modes to
	memory.
	(store_integral_bit_field): Defer unsplittable modes to
	extract_bit_field.
	(extract_bit_field_1): Spill if necessary.
	* expr.cc (read_complex_part): Adjust assert.
	* lower-subreg.cc (interesting_mode_p): Don't consider
	unsplittable modes interesting.
	(simple_move_operand): Exclude unsplittable inner modes.
---
 gcc/config/aarch64/aarch64.cc | 16 ++++++++--------
 gcc/config/riscv/riscv-v.cc   | 12 ++++--------
 gcc/config/riscv/riscv.cc     |  5 ++---
 gcc/emit-rtl.cc               | 13 +++++++++++++
 gcc/expmed.cc                 | 14 +++++++++++---
 gcc/expr.cc                   |  3 ---
 gcc/lower-subreg.cc           | 19 ++++++++++++++++++-
 7 files changed, 56 insertions(+), 26 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 3041a6ee62a..51635d5d08d 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -2995,20 +2995,20 @@ aarch64_regmode_natural_size (machine_mode mode)
   /* The natural size for SVE data modes is one SVE data vector,
      and similarly for predicates.  We can't independently modify
      anything smaller than that.  */
-  /* ??? For now, only do this for variable-width SVE registers.
-     Doing it for constant-sized registers breaks lower-subreg.cc.  */
-  /* ??? And once that's fixed, we should probably have similar
-     code for Advanced SIMD.  */
-  if (!aarch64_sve_vg.is_constant ())
-    {
-      /* REGMODE_NATURAL_SIZE influences general subreg validity rules,
-	 so we need to handle memory-only modes as well.  */
+  if (VECTOR_MODE_P (mode))
+    {
       unsigned int vec_flags = aarch64_classify_vector_memory_mode (mode);
       if (vec_flags & VEC_SVE_PRED)
 	return BYTES_PER_SVE_PRED;
       if (vec_flags & VEC_SVE_DATA)
 	return BYTES_PER_SVE_VECTOR;
+      if (vec_flags & VEC_ADVSIMD)
+	return MAX
+	  (exact_div (GET_MODE_SIZE (mode),
+		      aarch64_ldn_stn_vectors (mode)).to_constant (),
+	   UNITS_PER_WORD);
     }
+
   return UNITS_PER_WORD;
 }
 
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index bec1c9ebaec..4293cf2eef7 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -4057,14 +4057,10 @@ shuffle_even_odd_patterns (struct expand_vec_perm_d *d)
 
   /* When the element width is smaller than the greatest ELEN, we can use two
      vnsrl instructions, each extracting the even/odd elements of one source,
-     and a vslideup instruction to merge them into one vector.
-
-     PR target/124996: VLS mode subregs larger than what
-     riscv_regmode_natural_size allows cause a memory roundtrip.  Therefore, for
-     now, we only do this when the mode size is no greater than the natural size
-     of the register.  Once this is fixed, the condition should be replaced by
-     the ELEN condition.  */
-  if (known_le (GET_MODE_SIZE (vmode), riscv_regmode_natural_size (vmode)))
+     and a vslideup instruction to merge them into one vector.  */
+  unsigned int max_elen = TARGET_VECTOR_ELEN_64 ? 64 : 32;
+  if (known_le (GET_MODE_SIZE (vmode), riscv_regmode_natural_size (vmode))
+      && GET_MODE_BITSIZE (GET_MODE_INNER (vmode)) * 2 <= max_elen)
     {
       unsigned int elen = GET_MODE_BITSIZE (GET_MODE_INNER (vmode));
       unsigned int elen2x = elen * 2;
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 6593d00a7e6..168e100eb95 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -13357,9 +13357,6 @@ riscv_regmode_natural_size (machine_mode mode)
   /* The natural size for RVV data modes is one RVV data vector,
      and similarly for predicates.  We can't independently modify
      anything smaller than that.  */
-  /* ??? For now, only do this for variable-width RVV registers.
-     Doing it for constant-sized registers breaks lower-subreg.c.  */
-
   if (riscv_vector_mode_p (mode))
     {
       poly_uint64 size = GET_MODE_SIZE (mode);
@@ -13383,6 +13380,8 @@ riscv_regmode_natural_size (machine_mode mode)
 	   return minimum size between vector register size and scalar
 	   register size.  */
 	return MIN (size.to_constant (), UNITS_PER_WORD);
+      else
+	return TARGET_MIN_VLEN / UNITS_PER_WORD;
     }
   return UNITS_PER_WORD;
 }
diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
index 4a23eaefe02..e0f95f21ef5 100644
--- a/gcc/emit-rtl.cc
+++ b/gcc/emit-rtl.cc
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgrtl.h"
 #include "tree-eh.h"
 #include "explow.h"
+#include "expmed.h"
 #include "expr.h"
 #include "builtins.h"
 #include "rtl-iter.h"
@@ -1827,6 +1828,18 @@ operand_subword_force (rtx op, poly_uint64 offset, machine_mode mode)
 
   if (mode != BLKmode && mode != VOIDmode)
     {
+      /* If we have a subreg of an unsplittable register spill it before
+	 continuing.  */
+      if (SUBREG_P (op)
+	  && maybe_lt
+	  ((unsigned) UNITS_PER_WORD,
+	   (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (SUBREG_REG (op)))))
+	{
+	  poly_int64 size = GET_MODE_SIZE (GET_MODE (SUBREG_REG (op)));
+	  rtx mem = assign_stack_temp (GET_MODE (SUBREG_REG (op)), size);
+	  emit_move_insn (mem, SUBREG_REG (op));
+	  op = adjust_address (mem, mode, SUBREG_BYTE (op));
+	}
       /* If this is a register which cannot be accessed by words, copy it
 	 to a pseudo register.  */
       if (REG_P (op))
diff --git a/gcc/expmed.cc b/gcc/expmed.cc
index b87d06bc9a4..ae7b4ed896e 100644
--- a/gcc/expmed.cc
+++ b/gcc/expmed.cc
@@ -863,7 +863,10 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum,
       if (MEM_P (op0))
 	op0 = adjust_bitfield_address_size (op0, op0_mode.else_blk (),
 					    0, MEM_SIZE (op0));
-      else if (!op0_mode.exists ())
+      else if (!op0_mode.exists ()
+	       || maybe_lt
+	       ((unsigned) UNITS_PER_WORD,
+		(poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (op0))))
 	{
 	  if (ibitnum == 0
 	      && known_eq (ibitsize, GET_MODE_BITSIZE (GET_MODE (op0)))
@@ -1008,7 +1011,9 @@ store_integral_bit_field (rtx op0, opt_scalar_int_mode op0_mode,
 	      in BLKmode to handle unaligned memory references and to shift the
 	      last chunk right on big-endian machines if need be.  */
 	  rtx value_word
-	    = fieldmode == BLKmode
+	    = (fieldmode == BLKmode
+	       || maybe_lt ((unsigned) UNITS_PER_WORD,
+			    (poly_uint64) REGMODE_NATURAL_SIZE (value_mode)))
 	      ? extract_bit_field (value, new_bitsize, wordnum * BITS_PER_WORD,
 				   1, NULL_RTX, word_mode, word_mode, false,
 				   NULL)
@@ -1839,7 +1844,10 @@ extract_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum,
       if (MEM_P (op0))
 	op0 = adjust_bitfield_address_size (op0, op0_mode.else_blk (),
 					    0, MEM_SIZE (op0));
-      else if (op0_mode.exists (&imode))
+      else if (op0_mode.exists (&imode)
+	       && known_ge
+	       ((unsigned) UNITS_PER_WORD,
+		(poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (op0))))
 	{
 	  op0 = gen_lowpart (imode, op0);
 
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 0a7013e3a25..7397446047d 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -4199,9 +4199,6 @@ read_complex_part (rtx cplx, bool imag_p)
 				     imag_p ? GET_MODE_SIZE (imode) : 0);
       if (ret)
         return ret;
-      else
-	/* simplify_gen_subreg may fail for sub-word MEMs.  */
-	gcc_assert (MEM_P (cplx) && ibitsize < BITS_PER_WORD);
     }
 
   return extract_bit_field (cplx, ibitsize, imag_p ? ibitsize : 0,
diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc
index 5dee6a0b646..5033c6886c5 100644
--- a/gcc/lower-subreg.cc
+++ b/gcc/lower-subreg.cc
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "memmodel.h"
 #include "tm_p.h"
 #include "expmed.h"
+#include "regs.h"
 #include "insn-config.h"
 #include "emit-rtl.h"
 #include "recog.h"
@@ -113,6 +114,9 @@ interesting_mode_p (machine_mode mode, unsigned int *bytes,
 {
   if (!GET_MODE_SIZE (mode).is_constant (bytes))
     return false;
+  if (maybe_lt ((unsigned) UNITS_PER_WORD,
+		(poly_uint64) REGMODE_NATURAL_SIZE (mode)))
+    return false;
   *words = CEIL (*bytes, UNITS_PER_WORD);
   return true;
 }
@@ -302,7 +306,20 @@ static bool
 simple_move_operand (rtx x)
 {
   if (GET_CODE (x) == SUBREG)
-    x = SUBREG_REG (x);
+    {
+      /* Exclude subregs whose outer mode can be split into multiple words
+	 but whose inner mode cannot.  Attempting to split such a subreg
+	 would mean trying to split the unsplittable inner register.
+
+	 If instead the subreg occupies a single word, we can keep it as-is,
+	 regardless of what the SUBREG_REG is.  If the outer mode cannot be
+	 split then the subreg makes things no worse than they already are.  */
+      unsigned int factor, size;
+      if (interesting_mode_p (GET_MODE (x), &size, &factor) && factor > 1
+	  && !interesting_mode_p (GET_MODE (SUBREG_REG (x)), &size, &factor))
+	return false;
+      x = SUBREG_REG (x);
+    }
 
   if (!OBJECT_P (x))
     return false;
-- 
2.54.0