> Oops, I meant target-dependent of course.
Grml, now I know why the issues disappeared... I accidentally
re-introduced an older oversight in riscv's regmode_natural_size where I
divided by 64 instead of by 8, leading to very limited coverage.
I'll need at least two more hunks in expmed.cc and expr.cc both of which
would be better off checking for a mov optab as well. Also, there is an
ICE in simplify_subreg_concatn.
Rather than changing all callers of operand_subword_force, can we fix it
in there directly? Even though they are mostly clustered, all of them
would need to do the same thing anyway?
I think by now (with the approved changes) extract_bit_field and
store_bit_field can handle "word-unsplittable" vector modes already,
either by extract/set or by spilling, so we could use those?
With the changes below
- aarch64/riscv hooks
- accepted extract_bit_field/store_bit_field
- accepted read_complex_part
- your lower-subreg hunk
- operand_subword_force using extract_bit_field
- emit_move_multi_word using store_bit_field
on riscv I only have one lower-subreg ICE left:
FAIL: gcc.dg/pr44136.c (internal compiler error: in simplify_subreg_concatn, at
lower-subreg.cc:678)
Is that acceptable in order to avoid further whack-a-mole?
---
gcc/config/aarch64/aarch64.cc | 16 ++++++++--------
gcc/config/riscv/riscv-v.cc | 12 ++++--------
gcc/config/riscv/riscv.cc | 5 ++---
gcc/emit-rtl.cc | 9 +++++++++
gcc/expmed.cc | 10 ++++++++--
gcc/expr.cc | 19 +++++++++++++------
gcc/lower-subreg.cc | 19 ++++++++++++++++++-
7 files changed, 62 insertions(+), 28 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 93c00c23a50..a2d67ea3d39 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 01960ff96e5..fd4fd89d05d 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -4032,14 +4032,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 a401c0c7c93..a42fdc56e0e 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -13311,9 +13311,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);
@@ -13337,6 +13334,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..42d1b10090c 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,14 @@ operand_subword_force (rtx op, poly_uint64 offset,
machine_mode mode)
if (mode != BLKmode && mode != VOIDmode)
{
+ /* If the register cannot easily be split into words, let
+ extract_bit_field handle it. */
+ rtx tmp = SUBREG_P (op) ? SUBREG_REG (op) : op;
+ if (maybe_lt ((unsigned) UNITS_PER_WORD,
+ (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (tmp))))
+ return extract_bit_field (tmp, BITS_PER_WORD,
+ offset * BITS_PER_WORD, 1, NULL_RTX,
+ word_mode, word_mode, false, NULL);
/* 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..9c719d8614a 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)))
@@ -1839,7 +1842,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 210a7bc0888..7de94807550 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -4198,9 +4198,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,
@@ -4567,11 +4564,21 @@ emit_move_multi_word (machine_mode mode, rtx x, rtx y)
else if (ypart == 0)
ypart = operand_subword_force (y, i, mode);
- gcc_assert (xpart && ypart);
+ if (xpart == 0)
+ {
+ need_clobber = true;
+ store_bit_field (x, BITS_PER_WORD, i * BITS_PER_WORD, 0, 0,
+ word_mode, ypart, false, false);
+ last_insn = get_last_insn ();
+ }
+ else
+ {
+ gcc_assert (xpart && ypart);
- need_clobber |= (GET_CODE (xpart) == SUBREG);
+ need_clobber |= (GET_CODE (xpart) == SUBREG);
- last_insn = emit_move_insn (xpart, ypart);
+ last_insn = emit_move_insn (xpart, ypart);
+ }
}
seq = end_sequence ();
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
