On 26 September 2012 20:01, Richard Henderson <r...@twiddle.net> wrote:
> On 09/26/2012 11:48 AM, Peter Maydell wrote:
>> case INDEX_op_setcond_i32:
>> - if (const_args[2]) {
>> - int rot;
>> - rot = encode_imm(args[2]);
>> - tcg_out_dat_imm(s, COND_AL, ARITH_CMP, 0,
>> - args[1], rotl(args[2], rot) | (rot << 7));
>> - } else {
>> - tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0,
>> - args[1], args[2], SHIFT_IMM_LSL(0));
>> - }
>> + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0,
>> + args[1], args[2], const_args[2]);
>> tcg_out_dat_imm(s, tcg_cond_to_arm_cond[args[3]],
>> ARITH_MOV, args[0], 0, 1);
>> tcg_out_dat_imm(s, tcg_cond_to_arm_cond[tcg_invert_cond(args[3])],
>
> The patch itself is fine. But as a followup, if movcc is no longer "free",
> then perhaps the setcond sequence is better as
>
> cmp
> mov
> movcc
>
> i.e. the second move is unconditional? A register renaming OOO core could
> then schedule the mov before the cmp.
Well, maybe. But my bar for changing existing code requires more
proof that it's worth making the change (basically in both cases
I'm optimising for convenience of development). My suspicion
(entirely unbenchmarked) is that at the moment both these sequences
will pan out about the same cost, so we might as well pick the
one that's easiest to code.
-- PMM
