> -----Original Message-----
> From: Omar Tahir <omar.ta...@arm.com>
> Sent: 06 August 2020 13:02
> To: Kyrylo Tkachov <kyrylo.tkac...@arm.com>; ni...@redhat.com;
> Ramana Radhakrishnan <ramana.radhakrish...@arm.com>; Richard
> Earnshaw <richard.earns...@arm.com>; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH 3/5][Arm] New pattern for CSINC instructions
>
> > Hi Omar,
> >
> > diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
> > index 0b00aef7ef7..79cf684e5cb 100644
> > --- a/gcc/config/arm/thumb2.md
> > +++ b/gcc/config/arm/thumb2.md
> > @@ -743,6 +743,9 @@
> > if (GET_CODE (operands[4]) == LT && operands[3] == const0_rtx)
> > return \"%i5\\t%0, %1, %2, lsr #31\";
> >
> > + if (GET_CODE (operands[5]) == PLUS && TARGET_COND_ARITH)
> > + return \"cinc\\t%0, %1, %d4\";
> > +
> > output_asm_insn (\"cmp\\t%2, %3\", operands);
> >
> >
> > Hmmm, this looks wrong. The pattern needs to perform the comparison
> (setting the CC reg) as well as do the conditional increment.
> > Emitting a cinc without a cmp won't set the CC flags.
> > Also, cinc increments only by 1, whereas the "arm_rhs_operand" predicate
> accepts a wider variety of immediates, so just checking for GET_CODE
> (operands[5]) == PLUS isn't enough.
> >
> > Thanks,
> > Kyrill
> >
>
> My bad, the following line
>
> output_asm_insn (\"cmp\\t%2, %3\", operands);
>
> should be before my change rather than after, that will generate the cmp
> needed.
>
> As for the predicate accepting other immediates, I don't think that's an
> issue.
> From what I understand, the pattern represents
>
> r0 = f5 (f4 (r2, r3), r1)
>
> where f5 is a shiftable operator and f4 is a comparison operator. For
> simplicity let's just assume f4 is ==. Then we have
>
> r0 = f5 (r1, r2 == r3)
>
> If f5 is PLUS then we get
>
> r0 = r1 + (r2 == r3)
>
> which is
>
> r0 = (r2 == r3) ? r1 + 1 : r1
>
> i.e. cmp r2, r3 \\ cinc r0, r1, eq. Since all comparisons return either zero
> (comparison failed) or 1 (comparison passed) a cinc should always work as
> long as the shiftable operator is PLUS.
> Operand 3 being an "arm_rhs_operand" shouldn't matter since it's just being
> compared to operand 2 and returning a 0 or 1.
Ah, thanks for explaining, it's been a while since I messed with these
patterns...
Ok with the cmp fixed (as long as bootstrap and testing shows no problems)
>
> Thanks,
> Omar
