Segher Boessenkool <seg...@kernel.crashing.org> writes: > On Wed, Apr 14, 2021 at 05:31:23PM +0100, Richard Sandiford wrote: >> Jakub Jelinek <ja...@redhat.com> writes: >> > +(define_split >> > + [(set (match_operand:GPI 0 "register_operand") >> > + (LOGICAL:GPI >> > + (and:GPI (ashift:GPI (match_operand:GPI 1 "register_operand") >> > + (match_operand:QI 2 "aarch64_shift_imm_<mode>")) >> > + (match_operand:GPI 4 "const_int_operand")) >> > + (zero_extend:GPI (match_operand 3 "register_operand"))))] >> > + "can_create_pseudo_p () >> > + && REG_P (operands[1]) >> > + && REG_P (operands[3]) >> > + && REGNO (operands[1]) == REGNO (operands[3]) >> > + && ((unsigned HOST_WIDE_INT) >> > + trunc_int_for_mode (GET_MODE_MASK (GET_MODE (operands[3])) >> > + << INTVAL (operands[2]), <MODE>mode) >> > + == UINTVAL (operands[4]))" >> >> IMO this would be easier to understand as: >> >> && (trunc_int_for_mode (GET_MODE_MASK (GET_MODE (operands[3])) >> << INTVAL (operands[2]), <MODE>mode) >> == INTVAL (operands[4])) >> >> (At first I thought the cast and UINTVAL were trying to escape the >> sign-extension canonicalisation.) > > Yeah. Or just that with UINTVAL, the implicit conversion is fine (but > maybe that warns?) But INTVAL is simpler for sure. > >> I'm not sure about this one though. The REGNO checks mean that this is >> effectively for hard registers only. I thought one of the reasons for >> make_more_copies was to avoid combining hard registers like this, so I'm >> not sure we should have a pattern that specifically targets them. >> >> Segher, have I misunderstood? > > The REGNO checks work fine for pseudos as well. But, why does it do > this at all, instead of using match_dup? That should be clearer.
The register is appearing in two different modes: GPI for operand 1 and something smaller than GPI for operand 3 (otherwise the extension would be ill-formed). That's what makes it specific to hard registers. > The point of make_more_copies is that the hard registers from function > arguments are not pushed down by combine into actual instructions. This > can be done by RA if it thinks that is a good idea, and not done if it > thinks it is a bad idea. Having combine usurp part of the register > allocators role is not a good idea. > > There are other reasons hard regs can still end up in RTL insns in > earlier RTL passes of course, but the other changes that went together > with make_more_copies stop combine from doing that a lot (the function > itself makes sure every hard reg is copied to a new pseudo, because > combining that trivial move (from that new pseudo to the pseudo it was > copying it to already!) can still be beneficial for other reasons, all > strange and pretty unhappy, but important on many targets). What would your recommendation be for this pattern? Is matching hard registers a bad idea, or should we go with it? Thanks, Richard
