Re: [Qemu-devel] [PATCH 03/10] tcg: Swap commutative double-word comparisons

[Aurelien Jarno]

On Tue, Oct 09, 2012 at 08:31:47AM -0700, Richard Henderson wrote:
> On 10/09/2012 08:16 AM, Aurelien Jarno wrote:
> >> > +static bool swap_commutative2(TCGArg *p1, TCGArg *p2)
> >> > +{
> >> > +    int sum = 0;
> >> > +    sum += temps[p1[0]].state == TCG_TEMP_CONST;
> >> > +    sum += temps[p1[1]].state == TCG_TEMP_CONST;
> >> > +    sum -= temps[p2[0]].state == TCG_TEMP_CONST;
> >> > +    sum -= temps[p2[1]].state == TCG_TEMP_CONST;
> >> > +    if (sum > 0) {
> ...
> > Same comment are for the swap_commutative() patch, otherwise:
> 
> While I don't have an explicit test case for swap_commutative2 like
> I do for swap_commutative, think about how many conditionals you'd
> have to use to write this without using SUM:
> 
>   if (((temps[p1[0]].state == TCG_TEMP_CONST            // if both p1 are 
> const
>         && temps[p1[1]].state == TCG_TEMP_CONST
>         && !(temps[p2[0]].state == TCG_TEMP_CONST       // ... and not both 
> p2 are const
>              && temps[p2[1]].state == TCG_TEMP_CONST))
>       || ((temps[p1[0]].state == TCG_TEMP_CONST         // if either p1 are 
> const
>            || temps[p1[1]].state == TCG_TEMP_CONST)
>           && !temps[p2[0]].state == TCG_TEMP_CONST      // ... and neither p2 
> are const
>           && !temps[p2[1]].state == TCG_TEMP_CONST))
> 
> I don't see how that can possibly be easier to understand.
> 
> 

For that one I agree.

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