[v2] rs6000: Adjust mov optabs for opaque modes [PR103353]

  Hi,

As PR103353 shows, we may want to continue to expand a MMA built-in
function like a normal function, even if we have already emitted
error messages about some missing required conditions.  As shown in
that PR, without one explicit mov optab on OOmode provided, it would
call emit_move_insn recursively.

So this patch is to allow the mov pattern to be generated when we are
expanding to RTL and have seen errors even without MMA supported, it's
expected that the generated pattern would not cause further ICEs as the
compilation would stop soon after expanding.

Bootstrapped and regtested on powerpc64-linux-gnu P8 and
powerpc64le-linux-gnu P9 and P10.

v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591150.html

v2: Polish some comments and add one test case as Will and Peter suggested.

Is it ok for trunk or upcoming stage1?

BR,
Kewen
------

	PR target/103353

gcc/ChangeLog:

	* config/rs6000/mma.md (define_expand movoo): Move TARGET_MMA condition
	check to preparation statements and add handlings for !TARGET_MMA.
	(define_expand movxo): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.target/powerpc/pr103353.c: New test.
---
 gcc/config/rs6000/mma.md                    | 42 ++++++++++++++++++---
 gcc/testsuite/gcc.target/powerpc/pr103353.c | 22 +++++++++++
 2 files changed, 58 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103353.c

--
2.27.0
  

On Thu, 2022-04-07 at 17:29 +0800, Kewen.Lin wrote:
> Hi,
> 
> As PR103353 shows, we may want to continue to expand a MMA built-in
> function like a normal function, even if we have already emitted
> error messages about some missing required conditions.  As shown in
> that PR, without one explicit mov optab on OOmode provided, it would
> call emit_move_insn recursively.
> 
> So this patch is to allow the mov pattern to be generated when we are
> expanding to RTL and have seen errors even without MMA supported, it's
> expected that the generated pattern would not cause further ICEs as the
> compilation would stop soon after expanding.
> 
> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
> powerpc64le-linux-gnu P9 and P10.
> 
> v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591150.html
> 
> v2: Polish some comments and add one test case as Will and Peter suggested.

Thanks.

> 
> Is it ok for trunk or upcoming stage1?
> 
> BR,
> Kewen
> ------
> 
> 	PR target/103353
> 
> gcc/ChangeLog:
> 
> 	* config/rs6000/mma.md (define_expand movoo): Move TARGET_MMA condition
> 	check to preparation statements and add handlings for !TARGET_MMA.
> 	(define_expand movxo): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/powerpc/pr103353.c: New test.
> ---
>  gcc/config/rs6000/mma.md                    | 42 ++++++++++++++++++---
>  gcc/testsuite/gcc.target/powerpc/pr103353.c | 22 +++++++++++
>  2 files changed, 58 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103353.c
> 
> diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
> index 907c9d6d516..746a77a0957 100644
> --- a/gcc/config/rs6000/mma.md
> +++ b/gcc/config/rs6000/mma.md
> @@ -268,10 +268,25 @@ (define_int_attr avvi4i4i4	[(UNSPEC_MMA_PMXVI8GER4PP	"pmxvi8ger4pp")
>  (define_expand "movoo"
>    [(set (match_operand:OO 0 "nonimmediate_operand")
>  	(match_operand:OO 1 "input_operand"))]
> -  "TARGET_MMA"
> +  ""
>  {
> -  rs6000_emit_move (operands[0], operands[1], OOmode);
> -  DONE;
> +  if (TARGET_MMA) {
> +    rs6000_emit_move (operands[0], operands[1], OOmode);
> +    DONE;
> +  }
> +  /* Opaque modes are only expected to be available when MMA is supported,
> +     but PR103353 shows we may want to continue to expand a MMA built-in
> +     function, even if we have already emitted error messages about some
> +     missing required conditions.  As shown in that PR, without one
> +     explicit mov optab on OOmode provided, it would call emit_move_insn
> +     recursively.  So we allow this pattern to be generated when we are
> +     expanding to RTL and have seen errors, even though there is no MMA
> +     support.  It would not cause further ICEs as the compilation would
> +     stop soon after expanding.  */
> +  else if (currently_expanding_to_rtl && seen_error ())
> +    ;
> +  else
> +    gcc_unreachable ();
>  })

ok

> 
>  (define_insn_and_split "*movoo"
> @@ -300,10 +315,25 @@ (define_insn_and_split "*movoo"
>  (define_expand "movxo"
>    [(set (match_operand:XO 0 "nonimmediate_operand")
>  	(match_operand:XO 1 "input_operand"))]
> -  "TARGET_MMA"
> +  ""
>  {
> -  rs6000_emit_move (operands[0], operands[1], XOmode);
> -  DONE;
> +  if (TARGET_MMA) {
> +    rs6000_emit_move (operands[0], operands[1], XOmode);
> +    DONE;
> +  }
> +  /* Opaque modes are only expected to be available when MMA is supported,
> +     but PR103353 shows we may want to continue to expand a MMA built-in
> +     function, even if we have already emitted error messages about some
> +     missing required conditions.  As shown in that PR, without one
> +     explicit mov optab on XOmode provided, it would call emit_move_insn
> +     recursively.  So we allow this pattern to be generated when we are
> +     expanding to RTL and have seen errors, even though there is no MMA
> +     support.  It would not cause further ICEs as the compilation would
> +     stop soon after expanding.  */
> +  else if (currently_expanding_to_rtl && seen_error ())
> +    ;
> +  else
> +    gcc_unreachable ();
>  })

ok


> 
>  (define_insn_and_split "*movxo"
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103353.c b/gcc/testsuite/gcc.target/powerpc/pr103353.c
> new file mode 100644
> index 00000000000..6b0bedbb958
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr103353.c
> @@ -0,0 +1,22 @@
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* If the default cpu type is power10 or later, MMA is enabled by default.
> +   To keep the test point available all the time, this case specifies
> +   -mdejagnu-cpu=power6 to make it be tested without MMA.  */
> +/* { dg-options "-maltivec -mdejagnu-cpu=power6" } */
> +
> +/* Verify there is no ICE and don't check the error messages on MMA
> +   requirement since they could be fragile and are not test points
> +   of this case.  */
> +
> +void
> +foo (__vector_pair *dst, double *x)
> +{
> +  dst[0] = __builtin_vsx_lxvp (0, (__vector_pair *)(void *)x);
> +  /* { dg-error ".*" "pr103353" { target *-*-* } .-1 } */

ok

> +}
> +
> +void
> +bar (__vector_pair *src, double *x)
> +{
> +  __builtin_vsx_stxvp (src[0], 0, (__vector_pair *)(void *)x);
> +}


lgtm,
thanks
-Will

> --
> 2.27.0