Grml, setting REGMODE_NATURAL_SIZE for advsimd modes as well uncovered several
other lower-subreg issues and it, again, turned into a game of whack-a-mole and
obviously took way longer than I would have hoped.
(I hope the aarch64 changes are reasonable, needed to guard with VECTOR_MODE_P
or else we would consider large integer modes as struct modes?)
As far as I can tell just guarding the analysis phase in lower subreg is not
sufficient and we still try to split "not analyzed" uses/clobbers later.
That's why I needed to make changes to resolve_clobber, resolve_subreg_use, and
resolve_debug. I can't confidently say that the entire flow of lower subreg is
clear to me (hence whack-a-mole...)
Another small wart is that I encountered bootstrap failures on power because
regmode_natural_size is signed, while it is poly_uint64 on other targets.
Therefore, I just always cast both compared values.
The rest is mostly unchanged apart from more !MEM_P guards.
A follow-up patch is going to add the original function I wanted but I'll send
it separately.
Bootstrapped and regtested as before. There is one aarch64 ada testcase
(c394001) that has been flipping on and off. It might be an issue or might not
but I'll continue investigating. I still hope the diff is clean enough to
warrant a second look now.
Regards
Robin
[PATCH v2] expand: Handle non-word addressable 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.
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.
(emit_move_insn): Break infinite loop by spilling unsplittable
modes.
* lower-subreg.cc (can_decompose_p): Check if mode can be
split.
(interesting_mode_p): Don't consider unsplittable modes as
interesting.
(find_decomposable_subregs): Add unsplittable modes to
non_decomposable context.
(resolve_debug): Check.
---
gcc/config/aarch64/aarch64.cc | 13 +++-----
gcc/config/riscv/riscv-v.cc | 12 +++----
gcc/config/riscv/riscv.cc | 5 ++-
gcc/expmed.cc | 61 ++++++++++++++++++++++++++---------
gcc/expr.cc | 19 +++++++++--
gcc/lower-subreg.cc | 54 ++++++++++++++++++++++++++++---
6 files changed, 121 insertions(+), 43 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 72822ca621f..24b02780f7b 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -3040,20 +3040,17 @@ 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 exact_div (GET_MODE_SIZE (mode), aarch64_ldn_stn_vectors (mode));
}
+
return UNITS_PER_WORD;
}
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 431aaa1e761..4e5f011dc61 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -4011,14 +4011,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 d5eab542131..9e721366091 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -13302,9 +13302,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);
@@ -13328,6 +13325,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 / BITS_PER_WORD;
}
return UNITS_PER_WORD;
}
diff --git a/gcc/expmed.cc b/gcc/expmed.cc
index da1b5b63287..c4db5ef10bd 100644
--- a/gcc/expmed.cc
+++ b/gcc/expmed.cc
@@ -858,12 +858,16 @@ 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,
+ (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (op0))))
{
if (ibitnum == 0
&& known_eq (ibitsize, GET_MODE_BITSIZE (GET_MODE (op0)))
@@ -876,17 +880,29 @@ 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 (!MEM_P (op0)
+ && maybe_lt ((unsigned) UNITS_PER_WORD,
+ (poly_uint64) 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 +1024,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)
@@ -1834,12 +1852,16 @@ 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,
+ (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (op0))))
{
op0 = gen_lowpart (imode, op0);
@@ -1849,12 +1871,19 @@ 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 (!MEM_P (op0)
+ && maybe_lt ((unsigned) UNITS_PER_WORD,
+ (poly_uint64) 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 469c4d013b4..b5d06ab26ce 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -4176,9 +4176,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,
@@ -4714,6 +4711,22 @@ emit_move_insn (rtx x, rtx y)
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 at which point we'll try to decompose the
+ inner reg via operand_subword_force. If that fails we're back
+ here again without having made progress.
+ Therefore, spill Y to memory before we cannot escape anymore. */
+ rtx yreg = SUBREG_P (y) ? SUBREG_REG (y) : y;
+ if (optab_handler (mov_optab, mode) == CODE_FOR_nothing
+ && maybe_lt ((unsigned) UNITS_PER_WORD,
+ (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (yreg))))
+ {
+ rtx mem = assign_stack_temp (GET_MODE (yreg),
+ GET_MODE_SIZE (GET_MODE (yreg)));
+ emit_move_insn (mem, yreg);
+ y = adjust_address (mem, mode, 0);
+ }
+
if (CONSTANT_P (y))
{
if (optimize
diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc
index 5dee6a0b646..ed16e198d29 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"
@@ -104,6 +105,8 @@ struct target_lower_subreg *this_target_lower_subreg
#define choices \
this_target_lower_subreg->x_choices
+static bool can_decompose_p (rtx x);
+
/* Return true if MODE is a mode we know how to lower. When returning true,
store its byte size in *BYTES and its word size in *WORDS. */
@@ -113,6 +116,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;
}
@@ -510,9 +516,16 @@ find_decomposable_subregs (rtx *loc, enum
classify_move_insn *pcmi)
continue;
}
- if (!interesting_mode_p (GET_MODE (x), &outer_size, &outer_words)
- || !interesting_mode_p (GET_MODE (inner), &inner_size,
- &inner_words))
+ if (!interesting_mode_p (GET_MODE (x), &outer_size, &outer_words))
+ {
+ bitmap_set_bit (non_decomposable_context, regno);
+ bitmap_set_bit (subreg_context, regno);
+ iter.skip_subrtxes ();
+ continue;
+ }
+
+ if (!interesting_mode_p (GET_MODE (inner), &inner_size,
+ &inner_words))
continue;
/* We only try to decompose single word subregs of multi-word
@@ -775,7 +788,7 @@ resolve_subreg_use (rtx *loc, rtx insn)
{
rtx *loc = *iter;
rtx x = *loc;
- if (resolve_subreg_p (x))
+ if (resolve_subreg_p (x) && can_decompose_p (x))
{
x = simplify_subreg_concatn (GET_MODE (x), SUBREG_REG (x),
SUBREG_BYTE (x));
@@ -870,6 +883,19 @@ can_decompose_p (rtx x)
else
return !bitmap_bit_p (subreg_context, regno);
}
+ else if (SUBREG_P (x))
+ {
+ /* If X is a subreg, we need to check whether the outer mode or the
+ inner mode can be split into words. */
+ if (maybe_lt ((unsigned) UNITS_PER_WORD,
+ (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (x))))
+ return false;
+ rtx subreg = SUBREG_REG (x);
+ if (REG_P (subreg)
+ && maybe_lt ((unsigned) UNITS_PER_WORD,
+ (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (subreg))))
+ return false;
+ }
return true;
}
@@ -1184,6 +1210,24 @@ resolve_clobber (rtx pat, rtx_insn *insn)
return false;
orig_mode = GET_MODE (reg);
+
+ /* If we cannot decompose a subreg, clobber the concat's
+ individual elements directly and delete the original insn. */
+ if (SUBREG_P (reg) && !can_decompose_p (reg))
+ {
+ int start = SUBREG_BYTE (reg).to_constant () / UNITS_PER_WORD;
+ int nwords = GET_MODE_SIZE (orig_mode).to_constant () / UNITS_PER_WORD;
+ rtx concat = XEXP (reg, 0);
+ for (int i = start; i < start + nwords; i++)
+ {
+ rtx x = gen_rtx_CLOBBER (VOIDmode, XVECEXP (concat, 0, i));
+ emit_insn_after (x, insn);
+ }
+
+ delete_insn (insn);
+ return true;
+ }
+
if (!interesting_mode_p (orig_mode, &orig_size, &words))
gcc_unreachable ();
@@ -1236,7 +1280,7 @@ resolve_debug (rtx_insn *insn)
{
rtx *loc = *iter;
rtx x = *loc;
- if (resolve_subreg_p (x))
+ if (resolve_subreg_p (x) && can_decompose_p (x))
{
x = simplify_subreg_concatn (GET_MODE (x), SUBREG_REG (x),
SUBREG_BYTE (x));
--
2.54.0
