On Mon, Aug 05, 2019 at 11:41:41AM +0800, Kewen.Lin wrote:
> on 2019/8/4 上午4:52, Segher Boessenkool wrote:
> > On Fri, Aug 02, 2019 at 04:59:44PM +0800, Kewen.Lin wrote:
> >> As to the predicate name and usage, I checked the current vector shifts,
> >> they don't need to check const_vector specially (like right to left
> >> conversion), excepting for the one "vec_shr_<mode>", but it checks for
> >> scalar const int.
> >
> > I don't understand why we want to expand rotate-by-vector-of-immediates
> > if we have no insns for that? If you just use vint_operand, what happens
> > then?
>
> You are right, if we just use vint_operand, the functionality should be the
> same, the only small difference is the adjusted constant rotation number
> isn't masked, but it would be fine for functionality.
>
> One example for ULL >r 8, with const vector handling, it gets
> xxspltib 33,56
>
> Without the handling, it gets
> xxsplitb 33,248
>
> But I agree that it's trivial and unified it as below attached patch.
There are two cases: either all elements are rotated by the same amount,
or they are not. When they are, on p8 and later we can always use
xxspltib, which allows immediates 0..255, and the rotates look only at
the low bits they need, in that element, for that element (so you can
always splat it to all bytes, all but the low-order bytes are just
ignored by the rotate insns; before p8, we use vsplti[bhw], and those
allow -16..15, so for vrlw you do *not* want to mask it with 31.
There is some mechanics with easy_altivec_constant that should help
here. Maybe it can use some improvement.
The other case is if not all shift counts are the same. I'm not sure
we actually care much about this case :-)
> >> +/* { dg-options "-O3" } */
> >> +/* { dg-require-effective-target powerpc_altivec_ok } */
> >
> > If you use altivec_ok, you need to use -maltivec in the options, too.
> > This test should probably work with -O2 as well; use that, if possible.
>
> Sorry, the test case depends on vectorization which isn't enabled at -O2
> by default.
Ah yes, that is pretty obvious.
> >> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> >
> > I don't think we need this anymore? Not sure.
>
> I thought -mdejagnu-cpu=power8 can only ensure power8 cpu setting takes
> preference, but can't guarantee the current driver supports power8
> complication. As your comments, I guess since gcc configuration don't
> have without-cpu= etc., the power8 support should be always guaranteed?
The compiler always supports all CPUs.
If you are using something like -maltivec, things are different: you
might have selected a CPU that does not allow -maltivec, so we do need
altivec_ok. But if you use -mcpu=power8 (or -mdejagnu-cpu=power8), you
can use all p8 insns, including the vector ones (unless you disable
them again with -mno-vsx or similar; just don't do that ;-) )
[ In the past, it was possible to configure the compiler without support
for p8 vector insns, if your assembler doesn't support them. We do not
do this anymore: now, if your compiler does support things that your
assembler does not, you'll get errors from that assembler if you try to
use those instructions. Which is fine, just make sure you use a new
enough assembler for the GCC version you use. This always has been true,
but with a somewhat larger window of allowed versions. But this "don't
support all insns if the assembler does not" means we need to test a lot
more configurations (or leave them untested, even worse).
As a side effect, most *_ok now do nothing. *_hw of course is still
needed to check if the test system allows running the testcase. ]
> gcc/ChangeLog
>
> 2019-08-05 Kewen Lin <li...@gcc.gnu.org>
>
> * config/rs6000/vector.md (vrotr<mode>3): New define_expand.
>
> gcc/testsuite/ChangeLog
>
> 2019-08-05 Kewen Lin <li...@gcc.gnu.org>
>
> * gcc.target/powerpc/vec_rotate-1.c: New test.
> * gcc.target/powerpc/vec_rotate-2.c: New test.
> * gcc.target/powerpc/vec_rotate-3.c: New test.
> * gcc.target/powerpc/vec_rotate-4.c: New test.
Approved for trunk (with or without the p8vector_ok change). Thank you!
Segher
