On Fri, 28 Jun 2024, YunQiang Su wrote:

> > > >  Overall ISTM there is no need for distinct insns for ISA_HAS_COND_TRAPI
> > > > and !ISA_HAS_COND_TRAPI cases each and this would better be sorted with
> > > > predicates and constraints, especially as the output pattern is the same
> > > > in both cases anyway.  This would prevent special-casing from being 
> > > > needed
> > > > in `mips_expand_conditional_trap' as well.
> > > >
> > >
> > > I agree. The patch should be quite simple
> > >
> > >    [(trap_if (match_operator:GPR 0 "trap_comparison_operator"
> > >                                 [(match_operand:GPR 1 "reg_or_0_operand" 
> > > "dJ")
> > >                                  (match_operand:GPR 2 "arith_operand" 
> > > "dI")])
> > >             (const_int 0))]
> > >    "ISA_HAS_COND_TRAPI"
> > > -  "t%C0\t%z1,%2"
> > > +  "t%C0\t%z1,%z2"
> > >    [(set_attr "type" "trap")])
> >
> >  Nope, this is wrong.
> >
> 
> > in both cases anyway.  This would prevent special-casing from being needed
> > in `mips_expand_conditional_trap' as well.
> 
> We cannot make  `mips_expand_conditional_trap' simpler at this point.

 This is simply not true.  However as the platform maintainer you are the 
expert in this area, so I am leaving it to up you to figure out.  If you 
want, that is, of course.  All the necessary details are in the paragraph 
I've left quoted at the top.

 NB given that this is a fix for an easily reproducible bug, there should 
have been a test case committed along with it.

  Maciej

Reply via email to