Message ID | 87h8rzgjd8.fsf@linaro.org |
---|---|
State | New |
Headers | show |
Series | Restrict vector use of extract_bit_field_as_subreg (PR 83699) | expand |
On 01/06/2018 07:19 AM, Richard Sandiford wrote: > 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[234] 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? > > Richard > > > 2018-01-06 Richard Sandiford <richard.sandiford@linaro.org> > > gcc/ > PR rtl-optimization/83699 > * 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. THanks. Installed. jeff
Index: gcc/expmed.c =================================================================== --- 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 return target; } } - /* 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); if (sub)