Thanks for having a look.
> Why do we need to force it into a register? That can be
> counterproductive,
> since it means that both the subvector and the tuple are live at the same
> time. I realise we probably expect this to be optimised away later,
> but still, it feels a bit premature.
>
> I suppose most of extract_bit_field_1 is written assuming that:
>
> while (GET_CODE (op0) == SUBREG)
> {
> bitnum += SUBREG_BYTE (op0) * BITS_PER_UNIT;
> op0 = SUBREG_REG (op0);
> }
>
> has dealt with the subreg case, but given this change, I think it
> would be better to make the code more robust against SUBREGs instead.
> The while loop above managed to deal with subregs without emitting code,
> but doing that here would undo the optimisation.
>
> Or is forcing to a REG necessary for precisely that reason? Does keeping
> the SUBREG lead to inefficiencies later?
And once again we have a situation where an issue vanished :)
It definitely was necessary before but I tested without and everything
seems to work now. I don't recall the details but we would try subreg
shenanigans on the non-reg-forced subreg. I can only speculate but
perhaps we don't do this anymore now that REGMODE_NATURAL_SIZE is
defined properly.
I addressed the rest of your comments:
- Refactored extract_vector_from_vector into a "return early" style and
renamed variables.
- Adjusted comments.
- Put another extract_vector_from_vector after the multireg extract.
v2 which is currently being bootstrapped and regtested is below.
Regards
Robin
[PATCH v2] expmed: Improve tuple-extraction handling.
After the REGMODE_NATURAL_SIZE changes have landed, this is another small
improvement for PR125390 which also fixes aarch64's struct_3_256.c
regression.
Originally a riscv-specific issue and ICE, I noticed poor codegen when we
extract values from a tuple type. In the optimized dump we have:
vect_array.9 = .MASK_LEN_LOAD_LANES (&c, 32B, { -1, -1, -1, -1 }, _44(D), 4,
0);
_48 = BIT_FIELD_REF <vect_array.9[0], 32, 96>;
_50 = BIT_FIELD_REF <vect_array.9[1], 32, 96>;
f_13 = _48 ^ _50;
a = f_13;
and _48 as well as _50 spill to memory instead of extracting directly.
We end up with
(subreg:RVVMF2SI (reg:RVVMF2x2SI 138 [ vect_array.9 ]) 0)
which, as we pun the with a large integer mode, involves OImode
that we cannot "move". We might be able to work around that in the
target in unintuitive ways :) and we could also offer vec_extract for
all tuple modes. However, I figured it would be much easier if expand
already handled the tuple -> vector part for us so just a vector extract
is left for the target to do.
The patch adds multi-vector-reg -> vector functionality as a separate
function and also moves vector-vector, as well as vector-element extract into
new functions.
PR middle-end/125390
gcc/ChangeLog:
* expmed.cc (extract_bit_field_1): Call new functions.
(extract_vector_from_vector): New function.
(extract_element_from_vector): Likewise.
(extract_vector_from_multireg): Likewise.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/autovec/pr125390.c: Adjust test
expectation.
---
gcc/expmed.cc | 286 ++++++++++++------
.../gcc.target/riscv/rvv/autovec/pr125390.c | 4 +-
2 files changed, 202 insertions(+), 88 deletions(-)
diff --git a/gcc/expmed.cc b/gcc/expmed.cc
index c6494484251..7b6fcbe73cc 100644
--- a/gcc/expmed.cc
+++ b/gcc/expmed.cc
@@ -1698,101 +1698,83 @@ extract_bit_field_as_subreg (machine_mode mode, rtx
op0,
return NULL_RTX;
}
-/* A subroutine of extract_bit_field, with the same arguments.
- If UNSIGNEDP is -1, the result need not be sign or zero extended.
- If FALLBACK_P is true, fall back to extract_fixed_bit_field
- if we can find no other means of implementing the operation.
- if FALLBACK_P is false, return NULL instead. */
+/* Try to extract a subvector of mode SUBVEC_MODE (which has BITSIZE bits)
+ from vector VEC, starting at bit BITNUM, then extract the MODE lowpart of
+ the subvector. Return the result on success or NULL_RTX on failure. */
static rtx
-extract_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum,
- int unsignedp, rtx target, machine_mode mode,
- machine_mode tmode, bool reverse, bool fallback_p,
- rtx *alt_rtl)
+extract_vector_from_vector (rtx vec, rtx target, machine_mode mode,
+ machine_mode subvec_mode, poly_uint64 bitsize,
+ poly_uint64 bitnum, rtx *alt_rtl)
{
- rtx op0 = str_rtx;
- machine_mode mode1;
-
- if (tmode == VOIDmode)
- tmode = mode;
+ machine_mode vec_mode = GET_MODE (vec);
+ if (!VECTOR_MODE_P (vec_mode)
+ || MEM_P (vec)
+ || !VECTOR_MODE_P (subvec_mode)
+ || maybe_ne (bitsize, GET_MODE_PRECISION (subvec_mode))
+ || known_le (GET_MODE_SIZE (GET_MODE (vec)),
+ GET_MODE_SIZE (subvec_mode)))
+ return NULL_RTX;
- while (GET_CODE (op0) == SUBREG)
- {
- bitnum += SUBREG_BYTE (op0) * BITS_PER_UNIT;
- op0 = SUBREG_REG (op0);
+ /* Try to find a fitting mode for extracting. */
+ machine_mode punned_vec_mode = vec_mode;
+ if (GET_MODE_INNER (vec_mode) != GET_MODE_INNER (subvec_mode))
+ {
+ scalar_mode inner_mode = GET_MODE_INNER (subvec_mode);
+ poly_uint64 nunits;
+ if (!multiple_p (GET_MODE_BITSIZE (vec_mode),
+ GET_MODE_UNIT_BITSIZE (subvec_mode), &nunits)
+ || !related_vector_mode (subvec_mode, inner_mode,
+ nunits).exists (&punned_vec_mode)
+ || maybe_ne (GET_MODE_SIZE (punned_vec_mode),
+ GET_MODE_SIZE (vec_mode)))
+ return NULL_RTX;
}
+ vec_mode = punned_vec_mode;
- /* If we have an out-of-bounds access to a register, just return an
- uninitialized register of the required mode. This can occur if the
- source code contains an out-of-bounds access to a small array. */
- if (REG_P (op0) && known_ge (bitnum, GET_MODE_BITSIZE (GET_MODE (op0))))
- return gen_reg_rtx (tmode);
-
- if (REG_P (op0)
- && mode == GET_MODE (op0)
- && known_eq (bitnum, 0U)
- && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0))))
+ poly_uint64 pos;
+ enum insn_code icode;
+ if (((icode = convert_optab_handler
+ (vec_extract_optab, vec_mode, subvec_mode))
+ != CODE_FOR_nothing)
+ && multiple_p (bitnum, GET_MODE_BITSIZE (subvec_mode), &pos))
{
- if (reverse)
- op0 = flip_storage_order (mode, op0);
- /* We're trying to extract a full register from itself. */
- return op0;
- }
+ if (vec_mode != GET_MODE (vec))
+ vec = gen_lowpart (vec_mode, vec);
- /* First try to check for vector from vector extractions. */
- if (VECTOR_MODE_P (GET_MODE (op0))
- && !MEM_P (op0)
- && VECTOR_MODE_P (tmode)
- && known_eq (bitsize, GET_MODE_PRECISION (tmode))
- && maybe_gt (GET_MODE_SIZE (GET_MODE (op0)), GET_MODE_SIZE (tmode)))
- {
- machine_mode new_mode = GET_MODE (op0);
- if (GET_MODE_INNER (new_mode) != GET_MODE_INNER (tmode))
- {
- scalar_mode inner_mode = GET_MODE_INNER (tmode);
- poly_uint64 nunits;
- if (!multiple_p (GET_MODE_BITSIZE (GET_MODE (op0)),
- GET_MODE_UNIT_BITSIZE (tmode), &nunits)
- || !related_vector_mode (tmode, inner_mode,
- nunits).exists (&new_mode)
- || maybe_ne (GET_MODE_SIZE (new_mode),
- GET_MODE_SIZE (GET_MODE (op0))))
- new_mode = VOIDmode;
- }
- poly_uint64 pos;
- if (new_mode != VOIDmode
- && (convert_optab_handler (vec_extract_optab, new_mode, tmode)
- != CODE_FOR_nothing)
- && multiple_p (bitnum, GET_MODE_BITSIZE (tmode), &pos))
+ class expand_operand ops[3];
+ create_output_operand (&ops[0], target, subvec_mode);
+ ops[0].target = 1;
+ create_input_operand (&ops[1], vec, GET_MODE (vec));
+ create_integer_operand (&ops[2], pos);
+ if (maybe_expand_insn (icode, 3, ops))
{
- class expand_operand ops[3];
- machine_mode outermode = new_mode;
- machine_mode innermode = tmode;
- enum insn_code icode
- = convert_optab_handler (vec_extract_optab, outermode, innermode);
-
- if (new_mode != GET_MODE (op0))
- op0 = gen_lowpart (new_mode, op0);
- create_output_operand (&ops[0], target, innermode);
- ops[0].target = 1;
- create_input_operand (&ops[1], op0, outermode);
- create_integer_operand (&ops[2], pos);
- if (maybe_expand_insn (icode, 3, ops))
- {
- if (alt_rtl && ops[0].target)
- *alt_rtl = target;
- target = ops[0].value;
- if (GET_MODE (target) != mode)
- return gen_lowpart (tmode, target);
- return target;
- }
+ if (alt_rtl && ops[0].target)
+ *alt_rtl = target;
+ target = ops[0].value;
+ if (GET_MODE (target) != mode)
+ return gen_lowpart (subvec_mode, target);
+ return target;
}
}
+ return NULL_RTX;
+}
+
+/* Extract an element of OP0 of size BITSIZE, starting at bit position BITNUM.
+ Pun the result with mode TMODE if necessary. Return the extracted element
+ if successful or NULL_RTX otherwise. */
+
+static rtx
+extract_element_from_vector (rtx op0, rtx target, machine_mode mode,
+ machine_mode tmode, poly_uint64 bitsize,
+ poly_uint64 bitnum, rtx *alt_rtl)
+{
/* See if we can get a better vector mode before extracting. */
- if (VECTOR_MODE_P (GET_MODE (op0))
+ machine_mode outermode = GET_MODE (op0);
+ if (VECTOR_MODE_P (outermode)
&& !MEM_P (op0)
- && GET_MODE_INNER (GET_MODE (op0)) != tmode)
+ && GET_MODE_INNER (outermode) != tmode)
{
machine_mode new_mode;
@@ -1810,21 +1792,18 @@ extract_bit_field_1 (rtx str_rtx, poly_uint64 bitsize,
poly_uint64 bitnum,
new_mode = MIN_MODE_VECTOR_INT;
FOR_EACH_MODE_FROM (new_mode, new_mode)
- if (known_eq (GET_MODE_SIZE (new_mode), GET_MODE_SIZE (GET_MODE (op0)))
+ if (known_eq (GET_MODE_SIZE (new_mode), GET_MODE_SIZE (outermode))
&& known_eq (GET_MODE_UNIT_SIZE (new_mode), GET_MODE_SIZE (tmode))
&& known_eq (bitsize, GET_MODE_UNIT_PRECISION (new_mode))
&& multiple_p (bitnum, GET_MODE_UNIT_PRECISION (new_mode))
&& targetm.vector_mode_supported_p (new_mode)
- && targetm.modes_tieable_p (GET_MODE (op0), new_mode))
+ && targetm.modes_tieable_p (outermode, new_mode))
break;
if (new_mode != VOIDmode)
op0 = gen_lowpart (new_mode, op0);
}
- /* Use vec_extract patterns for extracting parts of vectors whenever
- available. If that fails, see whether the current modes and bitregion
- give a natural subreg. */
- machine_mode outermode = GET_MODE (op0);
+ outermode = GET_MODE (op0);
if (VECTOR_MODE_P (outermode) && !MEM_P (op0))
{
scalar_mode innermode = GET_MODE_INNER (outermode);
@@ -1866,6 +1845,139 @@ extract_bit_field_1 (rtx str_rtx, poly_uint64 bitsize,
poly_uint64 bitnum,
}
}
+ return NULL_RTX;
+}
+
+/* Check whether:
+
+ (a) OP0 is a vector that spans multiple registers (which might
+ mean that it is a tuple mode) and
+
+ (b) a sequence of BITSIZE bits, starting at bit *BITNUM, fits within
+ a single vector register.
+
+ Return the vector register if so and update *BITNUM so that it
+ is relative to the returned register. Return NULL_RTX otherwise. */
+
+static rtx
+extract_vector_from_multireg (rtx op0, machine_mode tmode,
+ poly_uint64 bitsize, poly_uint64 *bitnum)
+{
+ machine_mode outermode = GET_MODE (op0);
+ poly_uint64 outer_natural_sz = REGMODE_NATURAL_SIZE (outermode);
+ HOST_WIDE_INT vec_idx;
+ poly_uint64 off;
+ if (VECTOR_MODE_P (outermode)
+ && !MEM_P (op0)
+ && known_gt (GET_MODE_SIZE (outermode), outer_natural_sz)
+ && known_eq (bitsize, GET_MODE_PRECISION (tmode))
+ && known_le (GET_MODE_SIZE (tmode), outer_natural_sz)
+ && can_div_trunc_p (*bitnum, outer_natural_sz * BITS_PER_UNIT,
+ &vec_idx, &off)
+ && known_le (off + bitsize, outer_natural_sz * BITS_PER_UNIT))
+ {
+ /* Get a single-vector mode from the outer multi-vector mode.
+ If it exists and we can pun the outer mode with a subreg,
+ do so and return it. */
+ scalar_mode elmode = GET_MODE_INNER (outermode);
+ poly_uint64 nelts;
+ multiple_p (outer_natural_sz, GET_MODE_SIZE (elmode), &nelts);
+ opt_machine_mode vmode
+ = related_vector_mode (outermode, GET_MODE_INNER (outermode), nelts);
+
+ machine_mode vmode_onevec;
+ rtx subreg;
+ if (vmode.exists (&vmode_onevec)
+ && (subreg = simplify_gen_subreg
+ (vmode_onevec, op0, outermode,
+ vec_idx * GET_MODE_SIZE (vmode_onevec))))
+ {
+ *bitnum = off;
+ return subreg;
+ }
+ }
+
+ return NULL_RTX;
+}
+
+/* A subroutine of extract_bit_field, with the same arguments.
+ If UNSIGNEDP is -1, the result need not be sign or zero extended.
+ If FALLBACK_P is true, fall back to extract_fixed_bit_field
+ if we can find no other means of implementing the operation.
+ if FALLBACK_P is false, return NULL instead. */
+
+static rtx
+extract_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum,
+ int unsignedp, rtx target, machine_mode mode,
+ machine_mode tmode, bool reverse, bool fallback_p,
+ rtx *alt_rtl)
+{
+ rtx op0 = str_rtx;
+ machine_mode mode1;
+
+ if (tmode == VOIDmode)
+ tmode = mode;
+
+ while (GET_CODE (op0) == SUBREG)
+ {
+ bitnum += SUBREG_BYTE (op0) * BITS_PER_UNIT;
+ op0 = SUBREG_REG (op0);
+ }
+
+ /* If we have an out-of-bounds access to a register, just return an
+ uninitialized register of the required mode. This can occur if the
+ source code contains an out-of-bounds access to a small array. */
+ if (REG_P (op0) && known_ge (bitnum, GET_MODE_BITSIZE (GET_MODE (op0))))
+ return gen_reg_rtx (tmode);
+
+ if (REG_P (op0)
+ && mode == GET_MODE (op0)
+ && known_eq (bitnum, 0U)
+ && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0))))
+ {
+ if (reverse)
+ op0 = flip_storage_order (mode, op0);
+ /* We're trying to extract a full register from itself. */
+ return op0;
+ }
+
+ /* First try to check for vector from vector extractions. */
+ rtx res = extract_vector_from_vector (op0, target, mode, tmode, bitsize,
+ bitnum, alt_rtl);
+ if (res)
+ return res;
+
+ /* Use vec_extract patterns for extracting parts of vectors whenever
+ available. If that fails, see whether the current modes and bitregion
+ give a natural subreg. */
+ res = extract_element_from_vector (op0, target, mode, tmode, bitsize, bitnum,
+ alt_rtl);
+ if (res)
+ return res;
+
+ /* If this mode occupies multiple registers and we're extracting
+ a single one, we can usually get to that vector directly
+ without actual extraction.
+ This helps targets see through multi-vector modes. */
+ poly_uint64 bitnum_new = bitnum;
+ res = extract_vector_from_multireg (op0, tmode, bitsize, &bitnum_new);
+ if (res)
+ {
+ rtx vec = extract_vector_from_vector (res, target, mode, tmode, bitsize,
+ bitnum_new, alt_rtl);
+ if (vec)
+ return vec;
+
+ rtx el;
+ el = extract_element_from_vector (res, target, mode, tmode, bitsize,
+ bitnum_new, alt_rtl);
+ if (el)
+ return el;
+
+ op0 = res;
+ bitnum = bitnum_new;
+ }
+
/* Make sure we are playing with integral modes. Pun with subregs
if we aren't. */
opt_scalar_int_mode op0_mode = int_mode_for_mode (GET_MODE (op0));
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr125390.c
b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr125390.c
index 6ea9e509ba8..faa8eff767c 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr125390.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr125390.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-march=rv64gcv_zvl256b -mabi=lp64d -mrvv-vector-bits=zvl -O2
--param=riscv-autovec-mode=RVVMF8QI -fno-vect-cost-model" } */
+/* { dg-options "-march=rv64gcv_zvl256b -mabi=lp64d -mrvv-vector-bits=zvl -O2
--param=riscv-autovec-mode=RVVMF8QI -fno-vect-cost-model
-fdump-rtl-expand-details" } */
int a, b[64], c[64];
@@ -32,3 +32,5 @@ baz (void)
b[e] = 0;
}
}
+
+/* { dg-final { scan-rtl-dump-not "virtual-stack-vars" "expand" } } */
--
2.55.0
