RE: [PATCH, ARM] Generate conditional compares in Thumb2 state

["Jiangning Liu" <jiangning dot liu at arm dot com>]

Ramana,

I only see the following three combinations are meaningful, 

* cmp l, lPy // keep cmp, and length is 2, on thumb
* cmp r, rI  // keep cmp, and length is 4
* cmp r, L   // convert to cmn, and length is 4

According to ARM ARM, for negative immediate, all encodings for cmp/cmn are
4-byte long, i.e.

* CMP: encoding T2 and A1
* CMN: encoding T1 and A1

so we needn't to make difference to cover Pw and Pv.

> Length is 8 bytes if you have Pw and L

For this cases, the length should be 10 for Thumb2 instead. 

Finally, if we want to describe all possibilities in constraints, we would
have the followings 9 combinations,

* cmp1 has operands

(l,  l,  l,  r, r, r, r, r, r)
(lPy,lPy,lPy,rI,rI,rI,L, L, L)

* cmp2 has operands

(l,  r, r, l,  r, r, l,  r, r)
(lPy,rI,L, lPy,rI,L, lPy,rI,L)

So the length would be like below, (I don't know how to write it in
attribute section yet. )

if (TARGET_THUMB2) {
    (set_attr "length" "6,8,8,8,10,10,8,10,10")]
} else {
    (set_attr "length" "4,6,6,6,8,8,6,8,8")]
}

Does it make sense?

Thanks,
-Jiangning

> -----Original Message-----
> From: Ramana Radhakrishnan [mailto:ramana.radhakrishnan@linaro.org]
> Sent: Wednesday, August 10, 2011 6:40 PM
> To: Jiangning Liu
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
> 
> On 10 August 2011 09:20, Jiangning Liu <jiangning.liu@arm.com> wrote:
> > PING...
> 
> >
> > BTW, in patch fix_cond_cmp_2.patch, the file mode of thumb2.md is
> carelessly
> > changed, so please check attached new patch file fix_cond_cmp_3.patch.
> >
> 
> Please do not top post.
> 
> I've been away for the whole of last week and then been away from my
> desk
> for the first 2 days this week and am still catching up with email .
> 
> I'm missing a Changelog entry for this . Do I assume it is the same ?
> 
> I've just noticed that the length attribute isn't correct for Thumb2
> state. When I last
> looked at this I must have missed the case where the cmp and the cmn
> are both 32 bit instructions.
> 
> The length can vary between 6 and 10 bytes for the Thumb2 variant from
> my reading of the ARM-ARM.
> 
> i.e cmp reg, <8bitconstant>
>      it<cond>
>      cmn reg, <8bitconstant>
> 
> Length = 6 bytes
> 
> or even with
>    cmp reg, reg
>    it <cond>
>    cmn<cond> reg, reg
> 
>  All registers betwen r0 and r7 .
> 
> Length is 6 bytes if you have this for both l constraints.
> Length is 6 bytes - you should catch this with Pw and Pv respectively .
> Length is 8 bytes if you have Pw and L
> Length is 8 bytes if you have I and Pv
> Length is 10 bytes if you have I and L .
> 
> 
> Showing you an example with l and Py at this point of time and leaving
> the rest as homework. Yes it will duplicate a few cases but it helps
> getting the lengths absolutely right.
> 
> (define_insn "*cmp_ite0"
>   [(set (match_operand 6 "dominant_cc_register" "")
> 	(compare
> 	 (if_then_else:SI
> 	  (match_operator 4 "arm_comparison_operator"
> 	   [(match_operand:SI 0 "s_register_operand" "l,r,r,r,r")
> 	    (match_operand:SI 1 "arm_add_operand" "lPy,rI,L,rI,L")])
> 	  (match_operator:SI 5 "arm_comparison_operator"
> 	   [(match_operand:SI 2 "s_register_operand" "l,r,r,r,r")
> 	    (match_operand:SI 3 "arm_add_operand" "lPy,rI,rI,L,L")])
> 	  (const_int 0))
> 	 (const_int 0)))]
>   "TARGET_32BIT"
>   "*
>   {
>     static const char * const cmp1[5][2] =
>     {
>       {\"cmp\\t%2, %3\",
>        \"cmp\\t%0, %1\"},
>       {\"cmp\\t%2, %3\",
>        \"cmp\\t%0, %1\"},
>       {\"cmp\\t%2, %3\",
>        \"cmn\\t%0, #%n1\"},
>       {\"cmn\\t%2, #%n3\",
>        \"cmp\\t%0, %1\"},
>       {\"cmn\\t%2, #%n3\",
>        \"cmn\\t%0, #%n1\"}
>     };
>     static const char * const cmp2[5][2] =
>     {
>       {\"cmp%d5\\t%0, %1\",
>        \"cmp%d4\\t%2, %3\"},
>       {\"cmp%d5\\t%0, %1\",
>        \"cmp%d4\\t%2, %3\"},
>       {\"cmn%d5\\t%0, #%n1\",
>        \"cmp%d4\\t%2, %3\"},
>       {\"cmp%d5\\t%0, %1\",
>        \"cmn%d4\\t%2, #%n3\"},
>       {\"cmn%d5\\t%0, #%n1\",
>        \"cmn%d4\\t%2, #%n3\"}
>     };
>     static const char * const ite[2] =
>     {
>       \"it\\t%d5\",
>       \"it\\t%d4\"
>     };
>     int swap =
>       comparison_dominates_p (GET_CODE (operands[5]), GET_CODE
> (operands[4]));
> 
>     output_asm_insn (cmp1[which_alternative][swap], operands);
>     if (TARGET_THUMB2) {
>         output_asm_insn (ite[swap], operands);
>     }
>     output_asm_insn (cmp2[which_alternative][swap], operands);
>     return \"\";
>   }"
>   [(set_attr "conds" "set")
>    (set_attr "arch" "t2,any,any,any,any")
>    (set_attr "length" "6,8,8,8,8")]
> )
> 
> 
> >As for the extra problem exposed by this specific case, may we treat
> it as a
> >separate fix to decouple it with this one, and I can give follow up
> later
> >on? I think it is a general problem not only for the particular
> pattern
> >it/op/it/op. But I'm not sure how far we can go to optimize this kind
> of
> >problems introduced by IT block.
> 
> 
> Currently the way in which the Thumb2 backend generates
> conditional instructions and combines them further with other
> IT blocks is by running a state machine at the very end before assembly
> generation. Conditional instructions in GCC in RTL form usually have
> a cond-exec associated with them. The way this works is to look for
> sequences of cond-execs and inspect their conditions to merge
> them together later. Look at thumb2_final_prescan_insn in arm.c for
> more information. Mostly in the cases where we have such instructions
> especially where we have conditional compares being generated.
> 
> My suggestion in the previous mail about splitting this into cond-exec
> form
> instructions would help without changing too much of the current
> infrastructure.  I am happy to treat that as a follow-up set of patches.
> 
> cheers
> Ramana