On Mon, May 6, 2024 at 6:08 PM Richard Henderson
<richard.hender...@linaro.org> wrote:
> > - gen_update_cc_op(s);
> > l2 = gen_jz_ecx_string(s);
> > + /*
> > + * Only one iteration is done at a time, so there is
> > + * no control flow junction here and cc_op is never dynamic.
> > + */
> > fn(s, ot);
> > gen_op_add_reg_im(s, s->aflag, R_ECX, -1);
> > - gen_update_cc_op(s);
> > gen_jcc1(s, (JCC_Z << 1) | (nz ^ 1), l2);
> > if (s->repz_opt) {
> > gen_op_jz_ecx(s, l2);
>
> Ok, but only because gen_jcc1 does the gen_update_cc_op. The comment is
> neither correct
> nor necessary.
Yeah, it's true that gen_jcc1 does the update. On the other hand,
there are two different kinds of cc_op updates:
1) at branches, if you know that only one of the sides might write
cc_op - so you ensure it's up-to-date before the branch - and set
CC_OP_DYNAMIC at the junction. Same if you have movcond instead of a
branch.
2) at end of translation block, to spill the value lazily (because in
the middle of the TB we are able to restore it from insn data). With
these patches there is never a need to do this, because gen_jmp_rel()
and gen_jmp_rel_csize() take care of it.
The comment deals with the former, the removal with the latter.
The idea of the comment is that after SCAS/CMPS you have CC_OP_SUB*,
so in principle you may expect that you need to set CC_OP_DYNAMIC
explicitly at the end of a REPZ/REPNZ, which is where the CX != 0 and
CX == 0 paths join. But this is not necessary, because there is
nothing after that instruction - the translation block ends.
So I guess the comment could instead be placed at the end of the function?
/*
* Only one iteration is done at a time, so the translation
* block has ended unconditionally at this point and there
* is no control flow junction - no need to set CC_OP_DYNAMIC.
*/
What do you think?
Paolo
