Re: [PATCH, rs6000] Skip redundant vector extract if the element is first element of dword0 [PR110429]

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Haochen,

on 2023/7/5 11:22, HAO CHEN GUI wrote:
> Hi,
>   This patch skips redundant vector extract insn to be generated when
> the extracted element is the first element of dword0 and the destination

"The first element" is confusing, it's easy to be misunderstood as element
0, but in fact the extracted element index is: 
  - for byte, 7 on BE while 8 on LE;
  - for half word, 3 on BE while 4 on LE;

so maybe just say when the extracted index for byte and half word like above,
the element to be stored is already in the corresponding place for stxsi[hb]x,
we don't need a redundant vector extraction at all.

> is a memory operand. Only one 'stxsi[hb]x' instruction is enough.
> 
>   The V4SImode is fixed in a previous patch.
> https://gcc.gnu.org/pipermail/gcc-patches/2023-June/622101.html
> 
>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> Thanks
> Gui Haochen
> 
> ChangeLog
> rs6000: Skip redundant vector extract if the element is first element of
> dword0
> 
> gcc/
> 	PR target/110429
> 	* config/rs6000/vsx.md (*vsx_extract_<mode>_store_p9): Skip vector
> 	extract when the element is the first element of dword0.
> 
> gcc/testsuite/
> 	PR target/110429
> 	* gcc.target/powerpc/pr110429.c: New.
> 
> 
> patch.diff
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 0c269e4e8d9..b3fec910eb6 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -3855,7 +3855,22 @@ (define_insn_and_split "*vsx_extract_<mode>_store_p9"
>  		    (parallel [(match_dup 2)])))
>  	      (clobber (match_dup 4))])
>     (set (match_dup 0)
> -	(match_dup 3))])
> +	(match_dup 3))]
> +{
> +  enum machine_mode dest_mode = GET_MODE (operands[0]);

Nit: Move this line ...

> +
> +  if (which_alternative == 0
> +      && ((<MODE>mode == V16QImode
> +	   && INTVAL (operands[2]) == (BYTES_BIG_ENDIAN ? 7 : 8))
> +	  || (<MODE>mode == V8HImode
> +	      && INTVAL (operands[2]) == (BYTES_BIG_ENDIAN ? 3 : 4))))
> +    {

... here.

> +      emit_move_insn (operands[0],
> +		      gen_rtx_REG (dest_mode, REGNO (operands[3])));
> +      DONE;
> +    }
> +})
> +
> 
>  (define_insn_and_split  "*vsx_extract_si"
>    [(set (match_operand:SI 0 "nonimmediate_operand" "=r,wa,Z")
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110429.c b/gcc/testsuite/gcc.target/powerpc/pr110429.c
> new file mode 100644
> index 00000000000..5a938f9f90a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110429.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> +/* { dg-require-effective-target powerpc_p9vector_ok } */
> +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +
> +#include <altivec.h>
> +
> +#ifdef __BIG_ENDIAN__
> +#define DWORD0_FIRST_SHORT 3
> +#define DWORD0_FIRST_CHAR 7
> +#else
> +#define DWORD0_FIRST_SHORT 4
> +#define DWORD0_FIRST_CHAR 8
> +#endif
> +
> +void vec_extract_short (vector short v, short* p)
> +{
> +   *p = vec_extract(v, DWORD0_FIRST_SHORT);
> +}
> +
> +void vec_extract_char (vector char v, char* p)
> +{
> +   *p = vec_extract(v, DWORD0_FIRST_CHAR);
> +}
> +
> +/* { dg-final { scan-assembler-times "stxsi\[hb\]x" 2 } } */

Nit: Break this check into stxsihx and stxsibx, and surround
with \m and \M.

> +/* { dg-final { scan-assembler-not "vextractu\[hb\]" } } */

Also with \m and \M.

OK for trunk with these nits tweaked and testing goes well,
thanks!

BR,
Kewen