Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Laurent Desnogues]

On Mon, Dec 21, 2009 at 3:00 AM, Richard Henderson <address@hidden> wrote:
[...]
> I *am* convinced that to remove either VTRUE or VFALSE as arguments to the
> movcond primitive (implementing dest = (cond ? vtrue : dest) would do too
> much violence to both the liveness analysis and the register allocator
> within TCG.  The best I can do to remove the complexity is:
>
> static void tcg_out_movcond(TCGContext *s, int cond, TCGArg dest,
>                            TCGArg cmp1, TCGArg cmp2, int const_cmp2,
>                            TCGArg vtrue, int rexw)
> {
>    tcg_out_cmp(s, cond, cmp1, cmp2, const_cmp2, rexw);
>    /* cmovcc */
>    tcg_out_modrm(s, 0x40 | tcg_cond_to_jcc[cond] | P_EXT | rexw,
>                  dest, vtrue);
> }
> ...
>    { INDEX_op_movcond_i32, { "r", "r", "ri", "r", "0" } },
>    { INDEX_op_movcond_i64, { "r", "r", "re", "r", "0" } },
>
> using matching constraints in the target and directly implement the
> conditional move.  This eliminates the code I previously had that checked
> for things like dest=vfalse and inverted the condition.  I had planned to
> simplify the i386 version similarly, even in the case where cmovcc is not
> available.
>
> I would appreciate some direction here, so as to avoid wasting my time with
> a solution that won't be accepted.

The question for the generalized movcond is how useful is it?
Which front-ends would need it and would the cost to generate
code for it on some (most?) back-ends be amortized?

My guess (I use that word given that I didn't do any benchmark
to sustain my claim) is that your implementation is too complex.

Of course setcond can be implemented in terms of movcond,
but my guess (again that word...) is that setcond could be
enough and even faster in most cases.

To sum up, simplicity is extremely desirable as some profiles
are highly dependent on code generation time, and we lack
a set of benchmarks to check how good or bad our ideas
and implementations are.

Regarding your patches, I would like to see setcond put in
mainline with a simplified version for i386.  From there
people could implement it for all front-ends and back-ends.
Then we could move on to movcond and see how useful
it is.  I am all in favor of doing this incrementally as if we
aim for the best solution without participation from many
people, it'll be the best way not to do anything (cf. my
previous proposal for setcond more than a year ago).


Laurent