Re: [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal

[Richard Henderson]

On 09/16/2015 01:57 AM, Paolo Bonzini wrote:
> 
> 
> On 15/09/2015 20:45, Richard Henderson wrote:
>> +        /* Fold the global and local enable bits together into the
>> +           global fields, then xor to show which registers have
>> +           changed collective enable state.  */
>> +        int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 
>> 0xff;
> 
> The AND is not needed at all but, if you add it, you might as well use
> "& 0xaa" which is clearer.  But even better, just do:
> 
>    target_ulong old_dr7 = env->dr[7];
>    int mod = old_dr7 ^ new_dr7;
>    ...
>    if ((mod & ~0xff) == 0) {
> 
> 
> and test with
> 
>       if (mod & (3 << i * 2))
> 
> inside the loop.

Nope.  I wrote that the first time myself.  We're interested in two different
things: (1) whether or not something changed outside enable bits, and (2)
whether the enable state changed.

Since (2) is a combination of both global and local enable bits, we must
combine them *and then xor* to see if the enable state actually changes.  Just
using (mod & (3 << n)) will report "change" when local enable turns off, but
global enable remains on.  Which is not what we want.

Perhaps that comment could stand to be expanded...


r~