Hi Richard, Thank you for your comments. I've attached updated patch with changes reflecting your comments.
Kind regards,
Przemyslaw
> -----Original Message-----
> From: Richard Sandiford <richard.sandif...@arm.com>
> Sent: 19 August 2020 11:32
> To: Przemyslaw Wirkus <przemyslaw.wir...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> <richard.earns...@arm.com>; Marcus Shawcroft
> <marcus.shawcr...@arm.com>; Kyrylo Tkachov <kyrylo.tkac...@arm.com>
> Subject: Re: [PATCH PR96357][GCC][AArch64]: could not split insn
> UNSPEC_COND_FSUB with AArch64 SVE
>
> Przemyslaw Wirkus <przemyslaw.wir...@arm.com> writes:
> > Hi,
> >
> > Problem is related to that operand 4 (In original pattern
> > *cond_sub<mode>_any_const) is no longer the same as operand 1, and so
> > the pattern doesn't match the split condition.
> >
> > Pattern *cond_sub<mode>_any_const is being split by this patch into
> > two separate patterns:
> > * Pattern *cond_sub<mode>_relaxed_const now matches const_int
> > SVE_RELAXED_GP operand.
> > * Pattern *cond_sub<mode>_strict_const now matches const_int
> > SVE_STRICT_GP operand.
> > * Remove aarch64_sve_pred_dominates_p condition from both patterns.
>
> Thanks for doing this.
>
> > @@ -5271,6 +5270,43 @@ (define_insn_and_rewrite
> "*cond_sub<mode>_any_const"
> > [(set_attr "movprfx" "yes")]
> > )
> >
> > +;; Predicated floating-point subtraction from a constant, merging
> > +with an ;; independent value.
>
> The previous pattern had the same comment. Maybe add:
>
> The subtraction predicate and the merge predicate are allowed to be
> different.
>
> to the relaxed one and:
>
> The subtraction predicate and the merge predicate must be the same.
>
> to this one.
>
> > +(define_insn_and_rewrite "*cond_sub<mode>_strict_const"
> > + [(set (match_operand:SVE_FULL_F 0 "register_operand" "=w, w, ?w")
> > + (unspec:SVE_FULL_F
> > + [(match_operand:<VPRED> 1 "register_operand" "Upl, Upl, Upl")
> > + (unspec:SVE_FULL_F
> > + [(match_dup 1)
> > + (const_int SVE_STRICT_GP)
> > + (match_operand:SVE_FULL_F 2
> "aarch64_sve_float_arith_immediate")
> > + (match_operand:SVE_FULL_F 3 "register_operand" "w, w, w")]
> > + UNSPEC_COND_FSUB)
> > + (match_operand:SVE_FULL_F 4 "aarch64_simd_reg_or_zero" "Dz, 0,
> w")]
> > + UNSPEC_SEL))]
> > + "TARGET_SVE
> > + && !rtx_equal_p (operands[3], operands[4])"
>
> Very minor, but the file generally puts conditions on a single line if
> they'll fit.
> Same for the relaxed version.
>
> > + "@
> > +
> movprfx\t%0.<Vetype>, %1/z, %3.<Vetype>\;fsubr\t%0.<Vetype>, %1/m, %0.<
> Vetype>, #%2
> > +
> movprfx\t%0.<Vetype>, %1/m, %3.<Vetype>\;fsubr\t%0.<Vetype>, %1/m, %0.
> <Vetype>, #%2
> > + #"
> > + "&& 1"
> > + {
> > + if (reload_completed
> > + && register_operand (operands[4], <MODE>mode)
> > + && !rtx_equal_p (operands[0], operands[4]))
> > + {
> > + emit_insn (gen_vcond_mask_<mode><vpred> (operands[0],
> operands[3],
> > + operands[4], operands[1]));
> > + operands[4] = operands[3] = operands[0];
> > + }
> > + else if (!rtx_equal_p (operands[1], operands[5]))
> > + operands[5] = copy_rtx (operands[1]);
>
> The last two lines are a hold-over from the relaxed version, where there were
> two predicates. There's no operand 5 in this pattern, so we should just
> delete
> the lines.
>
> Thanks,
> Richard
rb13404.patch
Description: rb13404.patch
