This is the mail archive of the
mailing list for the GCC project.
Restrict vector use of extract_bit_field_as_subreg (PR 83699)
- From: Richard Sandiford <richard dot sandiford at linaro dot org>
- To: gcc-patches at gcc dot gnu dot org
- Date: Sat, 06 Jan 2018 14:19:31 +0000
- Subject: Restrict vector use of extract_bit_field_as_subreg (PR 83699)
- Authentication-results: sourceware.org; auth=none
The code in r256209 tried using subregs for two cases: to extract
a lowpart element of a vector, or to extract a subvector from a
wider vector. An earlier version of the SVE port used both variants,
but these days SVE provides vec_extract* for all cases and so only
really needs the subvector extract, e.g. for extracting one vector
from an LD result.
64-bit SPARC is unusual in that REGMODE_NATURAL_SIZE for vectors is
smaller than for GPRs. It's therefore valid to form a subreg reference
to both halves of a V2SI but not to both halves of a DI. This is where
the problem in PR83699 comes from: we extract the highpart SI from a V2SI
and want to move the result into a GPR. This causes LRA to cycle,
because it keeps emitting reload sequences of the form:
(set (reg:SI tmp) (subreg:SI (reg:V2SI foo) 0))
(set (reg:SI reg) (reg:SI tmp))
but then assigning a GPR to tmp, thus creating the same reload
problem as before.
However, even with that fixed, the sequence is worse than it was
before r256209. I don't think this means that using subregs is
a bad idea in principle, just that it needs a different condition.
(E.g. if SPARC supported V2SF, or if it was prepared to do the SImode
operation on FPRs, the patch would have been an improvement, because
we'd avoid doing the extraction via memory.)
But since there's no longer a motivating example for the single-element
case, I think the best approach is just to drop it. For the remaining
subvector case, we *could* continue to use the GET_MODE_INNER check
as well, but I don't think it's necessary, since what really matters
is whether the register reference can be formed.
Tested so far on aarch64-linux-gnu, and by spot-checking a
sparch64-linux-gnu cross. OK To install?
2018-01-06 Richard Sandiford <firstname.lastname@example.org>
* expmed.c (extract_bit_field_1): Restrict the vector usage of
extract_bit_field_as_subreg to cases in which the extracted
value is also a vector.
--- gcc/expmed.c 2018-01-06 10:55:43.333837784 +0000
+++ gcc/expmed.c 2018-01-06 14:15:01.179591726 +0000
@@ -1738,16 +1738,10 @@ extract_bit_field_1 (rtx str_rtx, poly_u
- /* Using subregs is useful if we're extracting the least-significant
- vector element, or if we're extracting one register vector from
- a multi-register vector. extract_bit_field_as_subreg checks
- for valid bitsize and bitnum, so we don't need to do that here.
- The mode check makes sure that we're extracting either
- a single element or a subvector with the same element type.
- If the modes aren't such a natural fit, fall through and
- bitcast to integers first. */
- if (GET_MODE_INNER (mode) == innermode)
+ /* Using subregs is useful if we're extracting one register vector
+ from a multi-register vector. extract_bit_field_as_subreg checks
+ for valid bitsize and bitnum, so we don't need to do that here. */
+ if (VECTOR_MODE_P (mode))
rtx sub = extract_bit_field_as_subreg (mode, op0, bitsize, bitnum);