Hi!
On Thu, Jul 27, 2017 at 07:21:14PM -0400, Michael Meissner wrote:
> This patches optimizes the PowerPC vector set operation for 64-bit doubles and
> longs where the elements in the vector set may have been extracted from
> another
> vector (PR target/81593):
> * config/rs6000/rs6000.c (rs6000_emit_xxpermdi): New function to
> emit XXPERMDI accessing either double word in either vector
> register inputs.
"emit" is not a good name for this: that is generally used for something
that does emit_insn, i.e. put an insn in the instruction stream. This
function returns a string a define_insn can return. For the rl* insns
I called the similar functions rs6000_insn_for_*, maybe something like
that is better here?
> +/* Emit a XXPERMDI instruction that can extract from either double word of
> the
> + two arguments. ELEMENT1 and ELEMENT2 are either NULL or they are 0/1
> giving
> + which double word to be used for the operand. */
> +
> +const char *
> +rs6000_emit_xxpermdi (rtx operands[], rtx element1, rtx element2)
> +{
> + int op1_dword = (!element1) ? 0 : INTVAL (element1);
> + int op2_dword = (!element2) ? 0 : INTVAL (element2);
> +
> + gcc_assert (IN_RANGE (op1_dword | op2_dword, 0, 1));
> +
> + if (BYTES_BIG_ENDIAN)
> + {
> + operands[3] = GEN_INT (2*op1_dword + op2_dword);
> + return "xxpermdi %x0,%x1,%x2,%3";
> + }
> + else
> + {
> + if (element1)
> + op1_dword = 1 - op1_dword;
> +
> + if (element2)
> + op2_dword = 1 - op2_dword;
> +
> + operands[3] = GEN_INT (op1_dword + 2*op2_dword);
> + return "xxpermdi %x0,%x2,%x1,%3";
> + }
> +}
I think calling this with the rtx elementN args makes this only more
complicated (the function comment doesn't say what they are or what
NULL means, btw).
> (define_insn "vsx_concat_<mode>"
> - [(set (match_operand:VSX_D 0 "gpc_reg_operand" "=<VSa>,we")
> + [(set (match_operand:VSX_D 0 "vsx_register_operand" "=wa,we")
> (vec_concat:VSX_D
> - (match_operand:<VS_scalar> 1 "gpc_reg_operand" "<VS_64reg>,b")
> - (match_operand:<VS_scalar> 2 "gpc_reg_operand" "<VS_64reg>,b")))]
> + (match_operand:<VS_scalar> 1 "gpc_reg_operand" "wa,b")
> + (match_operand:<VS_scalar> 2 "gpc_reg_operand" "wa,b")))]
> "VECTOR_MEM_VSX_P (<MODE>mode)"
> {
> if (which_alternative == 0)
> - return (BYTES_BIG_ENDIAN
> - ? "xxpermdi %x0,%x1,%x2,0"
> - : "xxpermdi %x0,%x2,%x1,0");
> + return rs6000_emit_xxpermdi (operands, NULL_RTX, NULL_RTX);
>
> else if (which_alternative == 1)
> - return (BYTES_BIG_ENDIAN
> + return (VECTOR_ELT_ORDER_BIG
> ? "mtvsrdd %x0,%1,%2"
> : "mtvsrdd %x0,%2,%1");
This one could be
{
if (!BYTES_BIG_ENDIAN)
std::swap (operands[1], operands[2]);
switch (which_alternative)
{
case 0:
return "xxpermdi %x0,%x1,%x2,0";
case 1:
return "mtvsrdd %x0,%1,%2";
default:
gcc_unreachable ();
}
}
(Could/should we use xxmrghd instead? Do all supported assemblers know
that extended mnemonic, is it actually more readable?)
> --- gcc/testsuite/gcc.target/powerpc/vsx-extract-7.c
> (svn+ssh://meiss...@gcc.gnu.org/svn/gcc/trunk/gcc/testsuite/gcc.target/powerpc/vsx-extract-7.c)
> (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/vsx-extract-7.c
> (.../gcc/testsuite/gcc.target/powerpc/vsx-extract-7.c) (revision 250640)
> @@ -0,0 +1,15 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O2 -mvsx" } */
> +
> +vector double
> +test_vpasted (vector double high, vector double low)
> +{
> + vector double res;
> + res[1] = high[1];
> + res[0] = low[0];
> + return res;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mxxpermdi\M} 1 } } */
In this and the other testcase, should you test no other insns at all
are generated?
Segher
