>> I suppose the question isn't so much "is this a tuple mode?" but more >> "does this vector occupy multiple registers?". REGMODE_NATURAL_SIZE >> seems like the right thing to test for that. The problem is that both >> AArch64 and RISC-V only define REGMODE_NATURAL_SIZE in the natural way >> for variable-length vectors: >> >> /* ??? For now, only do this for variable-width SVE registers. >> Doing it for constant-sized registers breaks lower-subreg.cc. */ >> >> So I suppose we'd need to fix that first (which would be a good thing, >> but a bit of tangent). > > This has bitten me several times already but I never bothered to investigate. > > Agreed that it would be a good thing to fix. I'll have a look (but not right > away). > >> Then related_vector_mode could be used to get the associated vector mode.
I couldn't resist even though I have tons of other things to do :/ But as one of my mid-term goals is to rework vec-vec extracts (for which we still rely on subregs surviving to the target), it seemed a reasonable fit. The attached patch/idea defines REGMODE_NATURAL_SIZE for VLS vectors as well and tries to handle the resulting fallout. At least for riscv, lower-subreg wasn't actually the problem but rather the arteries around emit_move, operand_force_subreg etc. Would an approach like the attached have any way forward? If so, a more thorough auditing of the various paths will be necessary. Obviously, the aarch64 changes are just ad-hoc. A previous version of it was bootstrapped/regtested as usual FWIW. Regards Robin >From d88e25212c5e585256061f5021ff860380438015 Mon Sep 17 00:00:00 2001 From: Robin Dapp <[email protected]> Date: Wed, 24 Jun 2026 12:02:46 +0200 Subject: [PATCH] expand: Handle non-"word splittable" modes. In order to be able to set REGMODE_NATURAL_SIZE = "one vector register" for aarch64 and riscv, we need to make some adjustments to how we expand modes that cannot be split into words. In a few critical spots the assumption of "either we have a move or we can split into words" was implicitly hard coded. There is also the infinite-loop case where emit_move_multi_word, itself a fallback for "split moving" calls calls operand_subword_force which goes to force_reg again that uses emit_move_multi_word again. It turned into a small game of whack-a-mole and this patch takes the approach of just spilling to memory if REGMODE_NATURAL_SIZE > UNITS_PER_WORD. In lower subreg we just don't decompose a register if the same condition applies. 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. (emit_move_insn): Break infinite loop by spilling unsplittable modes. * lower-subreg.cc (decompose_multiword_subregs): Do not decompose unsplittable modes. --- gcc/config/aarch64/aarch64.cc | 22 +++++------- gcc/config/riscv/riscv-v.cc | 4 ++- gcc/config/riscv/riscv.cc | 2 ++ gcc/expmed.cc | 63 ++++++++++++++++++++++++++--------- gcc/expr.cc | 31 +++++++++++++++-- gcc/lower-subreg.cc | 9 ++++- 6 files changed, 97 insertions(+), 34 deletions(-) diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index 9dd19d81d7b..350db7436ee 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -2952,20 +2952,14 @@ 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. */ - 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; - } + 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) && aarch64_sve_vg.is_constant ()) + return ordered_min (GET_MODE_SIZE (mode), (poly_uint64) UNITS_PER_VREG); + return UNITS_PER_WORD; } diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc index 431aaa1e761..3794f91c1cb 100644 --- a/gcc/config/riscv/riscv-v.cc +++ b/gcc/config/riscv/riscv-v.cc @@ -4018,7 +4018,9 @@ shuffle_even_odd_patterns (struct expand_vec_perm_d *d) 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))) + 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 853729f0646..927359d2d99 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -13303,6 +13303,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; } return UNITS_PER_WORD; } diff --git a/gcc/expmed.cc b/gcc/expmed.cc index da1b5b63287..3e005958839 100644 --- a/gcc/expmed.cc +++ b/gcc/expmed.cc @@ -858,12 +858,15 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, valid for integral modes. */ opt_scalar_int_mode op0_mode = int_mode_for_mode (GET_MODE (op0)); scalar_int_mode imode; + bool need_stack_p = false; if (!op0_mode.exists (&imode) || imode != GET_MODE (op0)) { 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, + REGMODE_NATURAL_SIZE (GET_MODE (op0)))) { if (ibitnum == 0 && known_eq (ibitsize, GET_MODE_BITSIZE (GET_MODE (op0))) @@ -876,17 +879,31 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, } 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, undefined_p); - emit_move_insn (op0, temp); - return true; + need_stack_p = true; } else op0 = gen_lowpart (op0_mode.require (), op0); } + else + { + if (maybe_lt ((unsigned) UNITS_PER_WORD, + REGMODE_NATURAL_SIZE (GET_MODE (op0)))) + need_stack_p = true; + } + + /* With or without punning we might be faced with a mode that we cannot + split into words. If so, spill OP0 to the stack and recurse. + This happens at most once. */ + if (need_stack_p) + { + 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, undefined_p); + emit_move_insn (op0, temp); + return true; + } return store_integral_bit_field (op0, op0_mode, ibitsize, ibitnum, bitregion_start, bitregion_end, @@ -1008,7 +1025,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, + REGMODE_NATURAL_SIZE (value_mode))) ? extract_bit_field (value, new_bitsize, wordnum * BITS_PER_WORD, 1, NULL_RTX, word_mode, word_mode, false, NULL) @@ -1834,12 +1853,15 @@ extract_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, if we aren't. */ opt_scalar_int_mode op0_mode = int_mode_for_mode (GET_MODE (op0)); scalar_int_mode imode; + bool need_stack_p = false; if (!op0_mode.exists (&imode) || imode != GET_MODE (op0)) { 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, + REGMODE_NATURAL_SIZE (GET_MODE (op0)))) { op0 = gen_lowpart (imode, op0); @@ -1849,12 +1871,21 @@ extract_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, op0 = force_reg (imode, op0); } else - { - poly_int64 size = GET_MODE_SIZE (GET_MODE (op0)); - rtx mem = assign_stack_temp (GET_MODE (op0), size); - emit_move_insn (mem, op0); - op0 = adjust_bitfield_address_size (mem, BLKmode, 0, size); - } + need_stack_p = true; + } + else + { + if (maybe_lt ((unsigned) UNITS_PER_WORD, + REGMODE_NATURAL_SIZE (GET_MODE (op0)))) + need_stack_p = true; + } + + if (need_stack_p) + { + poly_int64 size = GET_MODE_SIZE (GET_MODE (op0)); + rtx mem = assign_stack_temp (GET_MODE (op0), size); + emit_move_insn (mem, op0); + op0 = adjust_bitfield_address_size (mem, BLKmode, 0, size); } /* ??? We currently assume TARGET is at least as big as BITSIZE. diff --git a/gcc/expr.cc b/gcc/expr.cc index de73215ccc6..5e0c659cad6 100644 --- a/gcc/expr.cc +++ b/gcc/expr.cc @@ -4173,8 +4173,20 @@ read_complex_part (rtx cplx, bool imag_p) if (ret) return ret; else - /* simplify_gen_subreg may fail for sub-word MEMs. */ - gcc_assert (MEM_P (cplx) && ibitsize < BITS_PER_WORD); + { + /* Get the original reg we might be 'subreg'ing from. + If that has a vector mode we can fall back to + extract_bit_field. + + For scalar modes, simplify_gen_subreg may fail for + sub-word MEMs. */ + rtx reg = cplx; + if (SUBREG_P (reg)) + reg = SUBREG_REG (reg); + /* simplify_gen_subreg may fail for sub-word MEMs. */ + gcc_assert (VECTOR_MODE_P (GET_MODE (reg)) + || (MEM_P (cplx) && ibitsize < BITS_PER_WORD)); + } } return extract_bit_field (cplx, ibitsize, imag_p ? ibitsize : 0, @@ -4709,6 +4721,21 @@ emit_move_insn (rtx x, rtx y) y = y_inner; mode = GET_MODE (x_inner); } + /* If we don't have a move for MODE, emit_move_insn_1 will fall back to + emit_move_multi_word. The latter will call operand_subword_force, + which calls emit_move_insn again, without making progress. + Therefore, spill Y to memory and let the memory path below handle + it. */ + else if (optab_handler (mov_optab, mode) == CODE_FOR_nothing + && y_inner && !MEM_P (x) + && maybe_gt (REGMODE_NATURAL_SIZE (GET_MODE (y_inner)), + (unsigned) UNITS_PER_WORD)) + { + rtx mem = assign_stack_temp (GET_MODE (y_inner), + GET_MODE_SIZE (GET_MODE (y_inner))); + emit_move_insn (mem, y_inner); + y = adjust_address (mem, mode, 0); + } if (CONSTANT_P (y)) { diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc index 5dee6a0b646..59b99c34554 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" @@ -1636,7 +1637,13 @@ decompose_multiword_subregs (bool decompose_copies) bitmap_clear (sub_blocks); EXECUTE_IF_SET_IN_BITMAP (decomposable_context, 0, regno, iter) - decompose_register (regno); + { + if (maybe_lt + ((unsigned) UNITS_PER_WORD, + REGMODE_NATURAL_SIZE (GET_MODE (regno_reg_rtx[regno])))) + continue; + decompose_register (regno); + } FOR_EACH_BB_FN (bb, cfun) { -- 2.54.0
