Hi!
On Wed, Feb 26, 2020 at 05:32:23PM -0500, Michael Meissner wrote:
> What is happening is in some instances, we want to do a vector extract with a
> variable element where the vector is in a register:
>
> #include <altivec.h>
>
> long long
> foo (vector long long v, unsigned long n)
> {
> return vec_extract (v, n);
> }
>
> During the reload pass, the register allocator decides that it should spill
> the
> insn to the stack, and then do the vector extract from memory (which is an
> optimization to prevent loading the vector in case we only want one element).
And that causes LHS/SHL, very undesirable. Okay.
> Note, there is a 4th place that uses input_operand for variable vector
> extracts
> that is not touched by this patch.
There are more places that use input_operand. input_operand is meant to
be used for the RHS of mov patterns (with a reg as LHS), and it isn't
good to use it anywhere else: input_operand allows too much: *all*
memory, many constants, datums that can only go into some kinds of regs
(while you might have another kind).
In pretty much all cases reload/LRA can fix things, but you get worse
code that way.
rs6000_expand_vector_extract is always called with a register as second
operand, so the changes you made are safe.
The "reload_completed"s now have no function other than to force worse
code to be generated, you might want to do something about that as a
follow-up.
> -// P8 (LE) variables: addi,xxpermdi,mr,stxvd2x|stxvd4x,rldicl,sldi,ldx,blr
> -// P8 (BE) constants: mfvsrd
> -// P8 (BE) Variables: addi,xxpermdi,rldicl,mr,stxvd2x|stxvd4x,sldi,ldx,blr
> +// P8 (LE) variables: xori, rldic, mtvsrd, xxpermdi, vslo, mfvsrd
> +// P8 (BE) constants: xxpermdi, mfvsrd
> +// P8 (BE) Variables: rldic, mtvsrd, xxpermdi, vslo, mfvsrd
>
> -/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
> -/* { dg-final { scan-assembler-times {\maddi\M} 3 { target lp64 } } } */
> +/* results. */
> +/* { dg-final { scan-assembler-times {\mxori\M} 3 { target le } } } */
> +/* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 3 } } */
> +/* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstxvw4x\M} 4 { target
> ilp32 } } } */
> /* { dg-final { scan-assembler-times {\madd\M} 3 { target ilp32 } } } */
> /* { dg-final { scan-assembler-times {\mlwz\M} 11 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\maddi\M} 6 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\mmfvsrd\M} 6 { target lp64 } } } */
> +/* { dg-final { scan-assembler-times {\mmtvsrd\M} 3 { target lp64 } } } */
> /* { dg-final { scan-assembler-times {\mxxpermdi\M} 3 { target le } } } */
> +/* { dg-final { scan-assembler-times {\mxxpermdi\M} 6 { target { be && lp64
> } } } } */
> /* { dg-final { scan-assembler-times {\mxxpermdi\M} 2 { target { be && ilp32
> } } } } */
> -/* { dg-final { scan-assembler-times {\mxxpermdi\M} 3 { target { be && lp64
> } } } } */
> -/* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstxvw4x\M} 3 { target
> lp64 } } } */
> -/* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstxvw4x\M} 4 { target
> ilp32 } } } */
> -/* { dg-final { scan-assembler-times {\mrldicl\M|\mrldic\M|\mrlwinm\M} 3 } }
> */
> -/* { dg-final { scan-assembler-times {\mmfvsrd\M} 3 { target { lp64 } } } }
> */
> -/* { dg-final { scan-assembler-times {\mmfvsrd\M} 0 { target { be && ilp32 }
> } } } */
> -/* { dg-final { scan-assembler-times {\mmtvsrd\M} 0 { target { lp64 } } } }
> */
> -/* { dg-final { scan-assembler-times {\mmtvsrd\M} 0 { target { be && ilp32 }
> } } } */
> +/* { dg-final { scan-assembler-times {\mvslo\M} 3 { target lp64 } } } */
All this stays super fragile.
Okay for trunk. Thanks!
Segher
