On 16/09/2015 16:57, 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.
Ah, I see now. Perhaps int old_enable = (old_dr7 | (old_dr7 << 1)) & 0xaa; int new_enable = (new_dr7 | (new_dr7 << 1)) & 0xaa; ? > Perhaps that comment could stand to be expanded... I think at least for me it's just the expression that is too complex. Paolo