Tamar Christina <tamar.christ...@arm.com> writes: >> -----Original Message----- >> > + (match_operand:VQN 4 "register_operand" "w")))] >> > "TARGET_SIMD" >> > + "#" >> > + "&& true" >> > + [(const_int 0)] >> > { >> > - unsigned HOST_WIDE_INT size >> > - = (1ULL << GET_MODE_UNIT_BITSIZE (<VNARROWQ>mode)) - 1; >> > - rtx elt = unwrap_const_vec_duplicate (operands[2]); >> > - if (!CONST_INT_P (elt) || UINTVAL (elt) != size) >> > - FAIL; >> > - >> > - rtx addend = gen_reg_rtx (<MODE>mode); >> > - rtx val = aarch64_simd_gen_const_vector_dup (<VNARROWQ2>mode, 1); >> > - emit_move_insn (addend, lowpart_subreg (<MODE>mode, val, >> > <VNARROWQ2>mode)); >> > - rtx tmp1 = gen_reg_rtx (<VNARROWQ>mode); >> > - rtx tmp2 = gen_reg_rtx (<MODE>mode); >> > - emit_insn (gen_aarch64_addhn<mode> (tmp1, operands[1], addend)); >> > - unsigned bitsize = GET_MODE_UNIT_BITSIZE (<VNARROWQ>mode); >> > - rtx shift_vector = aarch64_simd_gen_const_vector_dup (<MODE>mode, >> > bitsize); >> > - emit_insn (gen_aarch64_uaddw<Vnarrowq> (tmp2, operands[1], tmp1)); >> > - emit_insn (gen_aarch64_simd_lshr<mode> (operands[0], tmp2, >> > shift_vector)); >> > + rtx tmp; >> > + if (can_create_pseudo_p ()) >> > + tmp = gen_reg_rtx (<VNARROWQ>mode); else >> > + tmp = gen_rtx_REG (<VNARROWQ>mode, REGNO (operands[0])); >> > + emit_insn (gen_aarch64_addhn<mode> (tmp, operands[1], >> operands[2])); >> > + emit_insn (gen_aarch64_uaddw<Vnarrowq> (operands[0], operands[4], >> > + tmp)); >> > DONE; >> > }) >> >> In the previous review, I said: >> >> However, IIUC, this pattern would only be formed from combining >> three distinct patterns. Is that right? If so, we should be able >> to handle it as a plain define_split, with no define_insn. >> That should make things simpler, so would be worth trying before >> the changes I mentioned above. >> >> Did you try that? I still think it'd be preferable to defining a new insn. > > Yes I did! Sorry I forgot to mention that. When I made it a split for some > reason It wasn't matching it anymore.
I was hoping for a bit more detail than that :-) But it seems that the reason is that we match SRA first, so the final combination is a 2-to-1 rather than 3-to-1. So yeah, the patch is OK with the other changes mentioned in the review. Thanks, Richard