From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36378) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZllOF-00032V-W5 for qemu-devel@nongnu.org; Mon, 12 Oct 2015 18:16:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZllOC-0000TP-Ol for qemu-devel@nongnu.org; Mon, 12 Oct 2015 18:16:19 -0400 Received: from mail-qg0-x22a.google.com ([2607:f8b0:400d:c04::22a]:35446) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZllOC-0000TH-KI for qemu-devel@nongnu.org; Mon, 12 Oct 2015 18:16:16 -0400 Received: by qgt47 with SMTP id 47so129920035qgt.2 for ; Mon, 12 Oct 2015 15:16:16 -0700 (PDT) Sender: Richard Henderson References: <1444643442-8487-1-git-send-email-pbonzini@redhat.com> From: Richard Henderson Message-ID: <561C3120.9080402@twiddle.net> Date: Tue, 13 Oct 2015 09:16:00 +1100 MIME-Version: 1.0 In-Reply-To: <1444643442-8487-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target-i386: fix pcmpxstrx equal-ordered (strstr) mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: fweimer@redhat.com, Eduardo Habkost On 10/12/2015 08:50 PM, Paolo Bonzini wrote: > In this mode, referring an invalid element of the source forces the > result to false (table 4-7, last column) but referring an invalid > element of the destination forces the result to true, so the outer > loop should still be run even if some elements of the destination > will be invalid. They will be culled in the inner loop, which > correctly bounds "i" to validd. > > This fix tst_strstr in glibc 2.17. > > Reported-by: Florian Weimer > Cc: Richard Henderson > Cc: Eduardo Habkost > Signed-off-by: Paolo Bonzini > --- > target-i386/ops_sse.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target-i386/ops_sse.h b/target-i386/ops_sse.h > index 7aa693a..268f3e1 100644 > --- a/target-i386/ops_sse.h > +++ b/target-i386/ops_sse.h > @@ -2037,7 +2037,7 @@ static inline unsigned pcmpxstrx(CPUX86State *env, Reg *d, Reg *s, > } > break; > case 3: > - for (j = valids - validd; j >= 0; j--) { > + for (j = valids; j >= 0; j--) { > res <<= 1; > v = 1; > for (i = MIN(upper - j, validd); i >= 0; i--) { I don't see how the bounding is properly done. In particular, > v &= (pcmp_val(s, ctrl, i + j) == pcmp_val(d, ctrl, i)); We're bounding j by valids, but accessing i+j? I think this would be a lot simpler if we simply followed the pseudocode in table 4-3, doing overrideIfDataInvalid after comparison. r~