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
