[PATCH PR96357][GCC][AArch64]: could not split insn UNSPEC_COND_FSUB with AArch64 SVE

Przemyslaw Wirkus Przemyslaw.Wirkus@arm.com
Tue Aug 25 08:56:28 GMT 2020


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.sandiford@arm.com>
> Sent: 19 August 2020 11:32
> To: Przemyslaw Wirkus <Przemyslaw.Wirkus@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: Re: [PATCH PR96357][GCC][AArch64]: could not split insn
> UNSPEC_COND_FSUB with AArch64 SVE
> 
> Przemyslaw Wirkus <Przemyslaw.Wirkus@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rb13404.patch
Type: application/octet-stream
Size: 4108 bytes
Desc: rb13404.patch
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200825/86924e81/attachment.obj>


More information about the Gcc-patches mailing list