Maciej W. Rozycki <ma...@orcam.me.uk> 于2024年6月28日周五 01:01写道：
>
> On Thu, 27 Jun 2024, YunQiang Su wrote:
>
> > > The missed optimisation in GAS, which used not to trigger pre-R6, is
> > > irrelevant from this change's point of view and just adds noise. I'm
> > > surprised that it worked even in the first place, as I reckon GCC is
> > > supposed to emit regular MIPS code in the `.set nomacro' mode nowadays,
> >
> > In fact, GCC works well if IMM is not zero in mips_expand_conditional_trap
> >
> > mode = GET_MODE (XEXP (comparison, 0));
> > op0 = force_reg (mode, op0);
> > if (!(ISA_HAS_COND_TRAPI
> > ? arith_operand (op1, mode)
> > : reg_or_0_operand (op1, mode)))
> > op1 = force_reg (mode, op1); // <--------- here
> >
> > This problem happens due to that GCC trust GAS so much ;)
> > It believe that GAS can recognize `TEQ $2,0`.
>
> Nope, the use of `reg_or_0_operand' (and the `J' constraint) implies the
> use of the `z' print operand modifier in the output template, there's no
> immediate operand expected to be ever produced from the output template in
> this case, which is the very bug (from commit 82f84ecbb47c ("MIPS32R6 and
> MIPS64R6 support") BTW) you have fixed.
>
> It is by pure chance that it worked before, because TEQ is an assembly
> macro (and `.set nomacro' should warn about it and with -Werror ultimately
In fact it doesn't work. I find this problem when I tried to fix some
GCC testcases.
> prevent assembly from succeeding) rather than a direct machine operation.
> It wouldn't have worked in the latter case at all (i.e. with some other
> instructions; there are existing examples in mips.md).
>
> > > 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.
As for pre-R6, we have TEQI, so that we can use it if IMM can be
represented with 16bit.
For R6 and IMM out range of 16bit, we have to emit more RTLs/INSNs to
load it into a reg.
Yes, we can merge the two template to
(define_insn "*conditional_trap<mode>"
[(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_TRAP"
{
if (!ISA_HAS_COND_TRAPI && !reg_or_0_operand(operands[2],
GET_MODE(operands[2])))
gcc_unreachable();
return "t%C0\t%z1,%z2";
}
[(set_attr "type" "trap")])
> Maciej
