Re: [Qemu-devel] [PATCH 3/5] target-arm: convert shl and shr helpers to TCG

[Aurelien Jarno <aurelien@aurel32.net>]

On Mon, Sep 17, 2012 at 11:47:05AM -0700, Richard Henderson wrote:
> On 09/17/2012 11:09 AM, Aurelien Jarno wrote:
> > If you insist, maybe we can just add a movcond op that uses setcond, so
> > it makes the code more readable?
> 
> Well, that's certainly a good first step.  And an easy fall back for
> hosts that choose not to implement the full conditional move.  But at
> least 4 of our 8 hosts do have a proper cmove insn.
> 
> But honestly I can't understand the resistance to movcond.
> 

I am not fundamentally opposed to a conditional mov instruction, that
said there are things I don't like in the way you implemented movcond.
When introducing new instructions, especially mandatory ones, we have
to really take care of designing it correctly, as changing it latter
won't be that easy and might requite an audit of all targets (look at
the shifts undefined result/behavior).

What I don't really like in the implementation you proposed:
- It's a mandatory op.
- It's implemented using jumps as a fallback at least on i386 and mips.
  One of the main reasons to have a movcond instruction, is to avoid
  jumps.
- I am not sure it really matches what's the host CPUs are providing,
  and also not sure it matches the needs. What you offer is a mix
  between isel and cmov.

One last argument is that in the patch series you offered, movcond
wasn't used a lot in the targets. Now that we might use it for
implementing shifts this argument has lost parts of its value.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net