From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54339) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcE92-0003f0-Jv for qemu-devel@nongnu.org; Wed, 16 Sep 2015 10:57:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZcE8z-0001ng-EK for qemu-devel@nongnu.org; Wed, 16 Sep 2015 10:57:12 -0400 Received: from mail-qk0-x22b.google.com ([2607:f8b0:400d:c09::22b]:36271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcE8z-0001na-74 for qemu-devel@nongnu.org; Wed, 16 Sep 2015 10:57:09 -0400 Received: by qkcf65 with SMTP id f65so87435241qkc.3 for ; Wed, 16 Sep 2015 07:57:08 -0700 (PDT) Sender: Richard Henderson References: <1442342713-29497-1-git-send-email-rth@twiddle.net> <1442342713-29497-5-git-send-email-rth@twiddle.net> <55F92F15.5020400@redhat.com> From: Richard Henderson Message-ID: <55F98340.1040801@twiddle.net> Date: Wed, 16 Sep 2015 07:57:04 -0700 MIME-Version: 1.0 In-Reply-To: <55F92F15.5020400@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: ehabkost@redhat.com 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~