On 10/06/2012 05:30 AM, Paolo Bonzini wrote:
> +static void gen_compute_eflags(DisasContext *s)
> {
> if (s->cc_op != CC_OP_DYNAMIC) {
> gen_op_set_cc_op(s->cc_op);
> }
> + if (s->cc_op == CC_OP_EFLAGS) {
> + return;
> }
> + gen_helper_cc_compute_all(cpu_tmp2_i32, cpu_env, cpu_cc_op);
> + tcg_gen_discard_tl(cpu_cc_dst);
> + s->cc_op = CC_OP_EFLAGS;
> + tcg_gen_extu_i32_tl(cpu_cc_src, cpu_tmp2_i32);
> }
Can we at this point in the series assert that if s->cc_op == CC_OP_EFLAGS,
then cpu_cc_op has also been assigned CC_OP_EFLAGS? If so, then we can do
if (s->cc_op == CC_OP_EFLAGS) {
return;
}
if (s->cc_op != CC_OP_DYNAMIC) {
gen_op_set_cc_op(s->cc_op);
}
...
As-is it would appear that we get redundant assignments to cpu_cc_op when
calling this routine twice in a row. And with that helper call in between
we won't be able to eliminate the second assignment.
I'll also note that we'd probably get better code if gen_helper_cc_compute_all
took all of cpu_cc_{op,src,dst} as arguments so that it could be CONST+PURE.
With just that changed I think the redundant assignment to cpu_cc_op would
be eliminated.
All that said, I don't see anything wrong with the patch as-is, and probably
these other things I mention would want to be follow-on patches anyway.
Reviewed-by: Richard Henderson <r...@twiddle.net>
r~
