Re: [PATCH] rs6000: Adjust mov optabs for opaque modes [PR103353]

[will schmidt <will_schmidt@vnet.ibm.com>]

On Thu, 2022-03-03 at 16:38 +0800, Kewen.Lin via Gcc-patches wrote:
> Hi,
> 

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.

Is there a testcase, new or existing, that illustrates this error path?

> 
> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
> powerpc64le-linux-gnu P9 and P10.
> 
> Is it ok for trunk?
> 
> 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/config/rs6000/mma.md | 42 ++++++++++++++++++++++++++++++++++------
> >  1 file changed, 36 insertions(+), 6 deletions(-)
> > 
> > diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
> > index 907c9d6d516..f76a87b4a21 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 like a normal function, even if we have already emitted
> > +     error messages about some missing required conditions.

perhaps drop "like a normal function".  


> > +     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.  */

Testcase would be particularly helpful to illustrate this, i think.  

TH
anks,
-Will

> > +  else if (currently_expanding_to_rtl && seen_error ())
> > +    ;
> > +  else
> > +    gcc_unreachable ();
> >  })
> >  
> >  (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 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 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 ();
> >  })
> >  
> >  (define_insn_and_split "*movxo"
> > -- 
> > 2.25.1
> >