Richard Earnshaw <richard.earns...@foss.arm.com> writes: > On 20/10/2023 13:13, Richard Sandiford wrote: >>> +(define_insn_and_split "*cmov<mode>_insn_insv" >>> + [(set (match_operand:GPI 0 "register_operand" "=r") >>> + (xor:GPI >>> + (neg:GPI >>> + (match_operator:GPI 1 "aarch64_comparison_operator" >>> + [(match_operand 2 "cc_register" "") (const_int 0)])) >>> + (match_operand:GPI 3 "general_operand" "r")))] >>> + "can_create_pseudo_p ()" >>> + "#" >>> + "&& true" > > >> IMO this is an ICE trap, since it hard-codes the assumption that there >> will be a split pass after the last pre-LRA call to recog. I think we >> should jsut provide the asm directly instead. > > So why not add > > (clobber (match_operand:GPI 4 "register_operand" "=&r")) > > to the insn, then you'll always get the scratch needed and the need to > check cane_create_pseudo_p goes away.
I think the "general_operand" "r" works in terms of ensuring that the source is a GPR. So we shouldn't need a separate clobber. Our off-list discussion made me realise that my concern above wasn't very clear. In principle, it should be possible for any pass to clear INSN_CODE and then rerecognise the pattern using recog. So I think it's wrong (or at least dangerous) for insns to require can_create_pseudo_p. It means that an insn starts out valid and suddenly becomes invalid half way through RTL compilation. But looking at it again, the patch seems correct with just the can_create_pseudo_p conditions removed. The constraints seem to satisfy what csinv requires, and the force_reg should be a no-op after RA. So the patch is OK with just the can_create_pseudo_p tests removed. Sorry for the run-around, and thanks for pushing back. Richard
