From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS17314 8.43.84.0/22 X-Spam-Status: No, score=-3.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_REPLYTO_END_DIGIT, HK_RANDOM_REPLYTO,MAILING_LIST_MULTI,PDS_RDNS_DYNAMIC_FP, RCVD_IN_DNSWL_MED,RDNS_DYNAMIC,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id BD56F1F403 for ; Wed, 5 Oct 2022 18:35:20 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="EdgY7UjW"; dkim-atps=neutral Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 85C1C3858024 for ; Wed, 5 Oct 2022 18:35:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 85C1C3858024 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1664994919; bh=VxoRSQe73UnR/lm+ASKD1t/ZBuPXNOCnUStEXRVPFng=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=EdgY7UjWFwI9OkFRzEBCAoLM50N+j3ID7QwKSTKCiuQxjkkkMr0+QyGbOCZgshrj3 Ercna9PSvwFJ1vnsBEdWLSiN/4avqCPXVCwcGAfSMyuZfmM8DjBJicTdL47a9AGlfQ 6wBMgvdB4pxTl5B8r8/yZvhs/ttjXPgYmB/DzIP0= Received: from mail-oi1-x22e.google.com (mail-oi1-x22e.google.com [IPv6:2607:f8b0:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id B11133858C2C; Wed, 5 Oct 2022 18:34:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B11133858C2C Received: by mail-oi1-x22e.google.com with SMTP id l5so18538715oif.7; Wed, 05 Oct 2022 11:34:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date; bh=VxoRSQe73UnR/lm+ASKD1t/ZBuPXNOCnUStEXRVPFng=; b=dnboBZbrd4dj/E8xaq7eBs08ryuq38NscpE94dowQ25YwhFgKmkf1X1vqmwHUaG5XV elmF0uI20FQsHZsApKbI7y+X5Vtq6blQ56tWAVaeYVmVV5hnkwLg56+9kXPrIsYUOnml HTdnGZXR8LdxRr72wHUXB2g/ZgopLGEJUNn5hJebZMx4mMQ7PSTOgZ1jA/TmUxyCCEq3 y9QmOqAoNqpziuYqnVcS+BdI22TukW3cSwmNAa74l829411ADKpmXW1/dsjSobNBKiAl 4vtEPHbr8x+bF4/ZEF+MiE6IKAYLL1jIZv5bzdfW0LY/Vjp+5ps1MwvCNbwiohDKLI9J JT5g== X-Gm-Message-State: ACrzQf3IggbRxn6fk6vwzcw6/xY+XX2D65Zg4V6QZI0UyzFH6ilhWSCC BkkjNACNzuIipMv+Is9i16qfwuBPAGSQtqcZxmagYkLM X-Google-Smtp-Source: AMsMyM7K3m++tbmW+x+VmxOd2mq1uhZezQ7mZlMEECOJbRPE4nWkn33u+lXiDsIrZZB6ngoHnygPVd7isBQJ2G0uoRI= X-Received: by 2002:a05:6808:1187:b0:353:a617:6acd with SMTP id j7-20020a056808118700b00353a6176acdmr572163oil.105.1664994894047; Wed, 05 Oct 2022 11:34:54 -0700 (PDT) MIME-Version: 1.0 References: <20210419233607.916848-1-goldstein.w.n@gmail.com> <20210419233607.916848-2-goldstein.w.n@gmail.com> In-Reply-To: Date: Wed, 5 Oct 2022 11:34:18 -0700 Message-ID: Subject: Re: [PATCH v5 2/2] x86: Optimize strlen-avx2.S To: Sunil Pandey , Noah Goldstein , Libc-stable Mailing List , Hongjiu Lu , GNU C Library Content-Type: text/plain; charset="UTF-8" X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sunil Pandey via Libc-alpha Reply-To: Sunil Pandey Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On Wed, Oct 5, 2022 at 10:11 AM Aurelien Jarno wrote: > > On 2022-10-04 18:10, Sunil Pandey via Libc-alpha wrote: > > On Tue, Oct 4, 2022 at 2:20 PM Aurelien Jarno wrote: > > > > > > On 2022-09-28 06:54, Sunil Pandey via Libc-stable wrote: > > > > Attached patch fixes BZ# 29611. > > > > > > > > I would like to backport it to 2.32,2.31,2.30,2.29 and 2.29. Let me know > > > > if there is any objection. > > > > > > Sorry to be late on this. I have a few comments about that patch: > > > > > > > From 86e1d88e1a3c126597ef39165275ada7564cfce9 Mon Sep 17 00:00:00 2001 > > > > From: "H.J. Lu" > > > > Date: Mon, 19 Apr 2021 10:45:07 -0700 > > > > Subject: [PATCH] x86-64: Require BMI2 for strchr-avx2.S > > > > > > > > Since strchr-avx2.S updated by > > > > > > > > commit 1f745ecc2109890886b161d4791e1406fdfc29b8 > > > > Author: noah > > > > Date: Wed Feb 3 00:38:59 2021 -0500 > > > > > > > > x86-64: Refactor and improve performance of strchr-avx2.S > > > > > > > > uses sarx: > > > > > > > > c4 e2 72 f7 c0 sarx %ecx,%eax,%eax > > > > > > > > for strchr-avx2 family functions, require BMI2 in ifunc-impl-list.c and > > > > ifunc-avx2.h. > > > > > > > > (cherry picked from commit 83c5b368226c34a2f0a5287df40fc290b2b34359) > > > > --- > > > > sysdeps/x86_64/multiarch/ifunc-avx2.h | 4 ++-- > > > > sysdeps/x86_64/multiarch/ifunc-impl-list.c | 12 +++++++++--- > > > > 2 files changed, 11 insertions(+), 5 deletions(-) > > > > > > First of all 1f745ecc2109890886b161d4791e1406fdfc29b8 never got > > > backported to 2.32 and older branches, and strchr-avx2.S in those > > > branches do not use BMI2 instructions. So it doesn't make sense to > > > backport it. > > > > > > That said the change in ifunc-avx2.h fixes: > > > > > > - memchr and rawmemchr, broken by the backport of acfd088a1963 ("x86: > > > Optimize memchr-avx2.S") > > > - strlen and strnlen, broken by the backport of aaa23c350715 ("x86: > > > Optimize strlen-avx2.S") > > > > > > So the issues are fixed, but mostly by chance. > > > > How do you know it is a "by chance" fix, do you have any evidence to back > > your claim? > > My point is that the commit that has been backported is fixing a bug > that doesn't exist in 2.32 branches. strchr-avx2.S does not the sarx > instruction as the commit claims, and does not use other BMI2 > instructions either. > > However following the backport of commit acfd088a1963 and aaa23c350715 > in these branches, memchr-avx2.S and strlen-avx2.S use BMI2 > instructions, and as they use ifunc-avx2.h, this actually fixes the bug. > This patch got selected because it fixes the ifunc-avx2.h file. My preference is to take an existing patch if possible, rather than creating a new one for branches. You are right, the original patch should have been composed differently to make it crystal clear. For backporting it's preferable to have small independent patches with logical grouping. > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurelien@aurel32.net http://www.aurel32.net