| Message ID | 533C1BAB.1020509@arm.com |
|---|---|
| State | New |
| Headers | show |
Sorry to be pedantic again, but 'wierd' should be spelt 'weird'. Otherwise, looks good to me and much neater than before. (Seems you'd rather keep the re-enabling, here and in the testsuite, for another patch?) --Alan Tejas Belagod wrote: > Richard Henderson wrote: >> On 02/21/2014 08:30 AM, Tejas Belagod wrote: >>> + /* If two vectors, we end up with a wierd mixed-endian mode on NEON. */ >>> + if (BYTES_BIG_ENDIAN) >>> + { >>> + if (!d->one_vector_p && d->perm[i] & nunits) >>> + { >>> + /* Extract the offset. */ >>> + elt = d->perm[i] & (nunits - 1); >>> + /* Reverse the top half. */ >>> + elt = nunits - 1 - elt; >>> + /* Offset it by the bottom half. */ >>> + elt += nunits; >>> + } >>> + else >>> + elt = nunits - 1 - d->perm[i]; >>> + } >> Isn't this just >> >> elt = d->perm[i] ^ (nunits - 1); >> >> all the time? I.e. invert the index within the word, >> but leave the word index (nunits) unchanged. >> > > Here is a revised patch. OK for stage-1? > > Thanks > Tejas. > > 2014-04-02 Tejas Belagod <tejas.belagod@yahoo.com> > > gcc/ > > * config/aarch64/aarch64.c (aarch64_evpc_tbl): Reverse order of elements > for big-endian. >
Alan Lawrence wrote: > Sorry to be pedantic again, but 'wierd' should be spelt 'weird'. Otherwise, > looks good to me and much neater than before. (Seems you'd rather keep the > re-enabling, here and in the testsuite, for another patch?) > Hi, Yes, the re-enabling is another patch. Thanks for the typo correction. OK for trunk with that change? Thanks, Tejas. > --Alan > > Tejas Belagod wrote: >> Richard Henderson wrote: >>> On 02/21/2014 08:30 AM, Tejas Belagod wrote: >>>> + /* If two vectors, we end up with a wierd mixed-endian mode on NEON. */ >>>> + if (BYTES_BIG_ENDIAN) >>>> + { >>>> + if (!d->one_vector_p && d->perm[i] & nunits) >>>> + { >>>> + /* Extract the offset. */ >>>> + elt = d->perm[i] & (nunits - 1); >>>> + /* Reverse the top half. */ >>>> + elt = nunits - 1 - elt; >>>> + /* Offset it by the bottom half. */ >>>> + elt += nunits; >>>> + } >>>> + else >>>> + elt = nunits - 1 - d->perm[i]; >>>> + } >>> Isn't this just >>> >>> elt = d->perm[i] ^ (nunits - 1); >>> >>> all the time? I.e. invert the index within the word, >>> but leave the word index (nunits) unchanged. >>> >> Here is a revised patch. OK for stage-1? >> >> Thanks >> Tejas. >> >> 2014-04-02 Tejas Belagod <tejas.belagod@yahoo.com> >> >> gcc/ >> >> * config/aarch64/aarch64.c (aarch64_evpc_tbl): Reverse order of elements >> for big-endian. >> >
On 22 April 2014 14:40, Tejas Belagod <tbelagod@arm.com> wrote: > Alan Lawrence wrote: >> >> Sorry to be pedantic again, but 'wierd' should be spelt 'weird'. >> Otherwise, >> looks good to me and much neater than before. (Seems you'd rather keep the >> re-enabling, here and in the testsuite, for another patch?) >> > > Hi, > > Yes, the re-enabling is another patch. > > Thanks for the typo correction. OK for trunk with that change? Yes that is OK. /Marcus
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index e839539..d30b79c 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -8129,7 +8129,15 @@ aarch64_evpc_tbl (struct expand_vec_perm_d *d) return false; for (i = 0; i < nelt; ++i) - rperm[i] = GEN_INT (d->perm[i]); + { + int nunits = GET_MODE_NUNITS (vmode); + + /* If big-endian and two vectors we end up with a wierd mixed-endian + mode on NEON. Reverse the index within each word but not the word + itself. */ + rperm[i] = GEN_INT (BYTES_BIG_ENDIAN ? d->perm[i] ^ (nunits - 1) + : d->perm[i]); + } sel = gen_rtx_CONST_VECTOR (vmode, gen_rtvec_v (nelt, rperm)); sel = force_reg (vmode, sel);