From patchwork Thu Nov 9 22:49:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Noah Goldstein X-Patchwork-Id: 1862269 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=a7pcOxVo; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4SRHG46rkDz1yQl for ; Fri, 10 Nov 2023 09:49:48 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BE3833858C60 for ; Thu, 9 Nov 2023 22:49:46 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com [IPv6:2a00:1450:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id 4B21C3858D33 for ; Thu, 9 Nov 2023 22:49:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4B21C3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4B21C3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::62c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699570177; cv=none; b=TjnLsnkAGW4cWzXYIrXQDFm85ZJ1eRdsiwlC0Ic7Ke1rnbMHen+TEUJk22dYuvIxksOk1MCW99ryBsGtfJ+crEe+GLRdHJoSFe82l1Pc/D0sOb1LGfU+s3DvN7lSpKy1PM0Ka83soZWX39yOTb89ZZYx+26kJGG/V5ehPMdipkw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699570177; c=relaxed/simple; bh=XgkELoanQOLxrlouhymwmLAUHLOhcpjv1v6S6rgN37o=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=wz+Uw/fI3gZP90Kln+44x1TFp4Dqw3NsNY7tiipXd+rVlU6FDGUjdVzTyl5PQZF3vWf/Ej16fTb1fvBKlkc+QTlPwhaUCDP0m1acRTdPByND4IMnXeawFByr28XvcrYJfcSeTo29LujsUgTYgh1oEFTHWleyy7vez/9LlXOjNxc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ej1-x62c.google.com with SMTP id a640c23a62f3a-9e61e969b1aso7618366b.0 for ; Thu, 09 Nov 2023 14:49:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699570173; x=1700174973; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=forUruxDOFP068IGo9+U88FnwGUCLUOhaCzve/xGwfo=; b=a7pcOxVoo7uMbq7MyrwMVwTOAkcTPM30pQXl0J3/cFRvAD/Ih0LqyuJMZQPz3/NY7Q Vrxw8vuiOG8UDePD0wveZdzf1bD8c8aAGNrSdJH3MKTWNszagn0nznl047AHWmUmXl9i pIA5FN/Yd3jxHJ/ZjhK3OOhQhpecVRS8uujkHPlc02KrQodzfOfgTh5eZoR6ixTwfty3 mcuDMaCn9H4EuNbqIy8ny8Zpfv6R4YSOQ0TH0izicKTwSbBQM08tidlo8ebJl3sN+x1d f3S0aJ83t6m8ErD8DWM88u6f/T9Llbnrq7Md6PhUrWbdN6prYaRUh4XsQGB6aTegMy8L 2Y/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699570173; x=1700174973; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=forUruxDOFP068IGo9+U88FnwGUCLUOhaCzve/xGwfo=; b=KAtwIqBLv4hFTb+Zp/E2qXRdqdm3Mj26uaVTz/5wKCY87pPKrMebF+6dnVg3haVzUI q+tpS6uMNcpid39XraD/0hIJaTbbjrQOZNSl0V+1+MixhWuPSAANW4f1DHiBw0mrwk80 p+DHWczkYUscO7Rd9+p+qCqYt/QqRc0S+eu/QC6/pU6h/igIq+SMBCVw/h4sTzOO13RB swT7nyJiGtPaXYzUKkL3+9BGQCwUjG1tJ6vhaEMpkXO2d49VNvVMNq00+vEBoQm+TjiL l39csHJsQQ+2I7zcGoVfligaLLXcmQDdHHJx8Xn5osTZqtPESFGOhNDyfF96Plf4TJpC KMvg== X-Gm-Message-State: AOJu0Yy4kkSJKlETZD3eGxwrH+pWe7pUsbd7TK6Ruwy4JVOKhuWBSZeV ZC4+uNIAQsQnZt6ejStgJD/H2TsgryY= X-Google-Smtp-Source: AGHT+IExeiZZk0suVMHToLfrrWjC+SCNdYpjhN8D7TpZuIxxSu/JZmG3xyJgao/xLt2MYWkFiYjd+w== X-Received: by 2002:a17:906:dacb:b0:9c7:59d1:b2c2 with SMTP id xi11-20020a170906dacb00b009c759d1b2c2mr5078758ejb.27.1699570173122; Thu, 09 Nov 2023 14:49:33 -0800 (PST) Received: from noahgold-desk.intel.com ([192.55.55.59]) by smtp.gmail.com with ESMTPSA id i9-20020a1709064ec900b009a5f1d15642sm3106380ejv.158.2023.11.09.14.49.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Nov 2023 14:49:32 -0800 (PST) From: Noah Goldstein To: libc-alpha@sourceware.org Cc: goldstein.w.n@gmail.com, hjl.tools@gmail.com, carlos@systemhalted.org Subject: [PATCH v3] x86: Fix unchecked AVX512-VBMI2 usage in strrchr-evex-base.S Date: Thu, 9 Nov 2023 16:49:18 -0600 Message-Id: <20231109224918.2036632-1-goldstein.w.n@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231101221657.311121-1-goldstein.w.n@gmail.com> References: <20231101221657.311121-1-goldstein.w.n@gmail.com> MIME-Version: 1.0 X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org strrchr-evex-base used `vpcompress{b|d}` in the page cross logic but was missing the CPU_FEATURE checks for VBMI2 in the ifunc/ifunc-impl-list. The fix is either to add those checks or change the logic to not use `vpcompress{b|d}`. Choosing the latter here so that the strrchr-evex implementation is usable on SKX. New implementation is a bit slower, but this is in a cold path so its probably okay. --- sysdeps/x86_64/multiarch/strrchr-evex-base.S | 61 ++++++++++++-------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/sysdeps/x86_64/multiarch/strrchr-evex-base.S b/sysdeps/x86_64/multiarch/strrchr-evex-base.S index cd6a0a870a..eb5f1f855a 100644 --- a/sysdeps/x86_64/multiarch/strrchr-evex-base.S +++ b/sysdeps/x86_64/multiarch/strrchr-evex-base.S @@ -35,7 +35,6 @@ # define CHAR_SIZE 4 # define VPCMP vpcmpd # define VPMIN vpminud -# define VPCOMPRESS vpcompressd # define VPTESTN vptestnmd # define VPTEST vptestmd # define VPBROADCAST vpbroadcastd @@ -46,7 +45,6 @@ # define CHAR_SIZE 1 # define VPCMP vpcmpb # define VPMIN vpminub -# define VPCOMPRESS vpcompressb # define VPTESTN vptestnmb # define VPTEST vptestmb # define VPBROADCAST vpbroadcastb @@ -71,7 +69,7 @@ ENTRY_P2ALIGN(STRRCHR, 6) andl $(PAGE_SIZE - 1), %eax cmpl $(PAGE_SIZE - VEC_SIZE), %eax jg L(cross_page_boundary) - +L(page_cross_continue): VMOVU (%rdi), %VMM(1) /* k0 has a 1 for each zero CHAR in YMM1. */ VPTESTN %VMM(1), %VMM(1), %k0 @@ -79,10 +77,11 @@ ENTRY_P2ALIGN(STRRCHR, 6) test %VGPR(rsi), %VGPR(rsi) jz L(aligned_more) /* fallthrough: zero CHAR in first VEC. */ -L(page_cross_return): + /* K1 has a 1 for each search CHAR match in VEC(1). */ VPCMPEQ %VMATCH, %VMM(1), %k1 KMOV %k1, %VGPR(rax) +L(page_cross_return): /* Build mask up until first zero CHAR (used to mask of potential search CHAR matches past the end of the string). */ blsmsk %VGPR(rsi), %VGPR(rsi) @@ -167,7 +166,6 @@ L(first_vec_x1_return): .p2align 4,, 12 L(aligned_more): -L(page_cross_continue): /* Need to keep original pointer incase VEC(1) has last match. */ movq %rdi, %rsi andq $-VEC_SIZE, %rdi @@ -340,34 +338,49 @@ L(return_new_match_ret): leaq (VEC_SIZE * 2)(%rdi, %rax, CHAR_SIZE), %rax ret - .p2align 4,, 4 L(cross_page_boundary): + /* eax contains all the page offset bits of src (rdi). `xor rdi, + rax` sets pointer will all page offset bits cleared so + offset of (PAGE_SIZE - VEC_SIZE) will get last aligned VEC + before page cross (guaranteed to be safe to read). Doing this + as opposed to `movq %rdi, %rax; andq $-VEC_SIZE, %rax` saves + a bit of code size. */ xorq %rdi, %rax - mov $-1, %VRDX - VMOVU (PAGE_SIZE - VEC_SIZE)(%rax), %VMM(6) - VPTESTN %VMM(6), %VMM(6), %k0 + VMOVU (PAGE_SIZE - VEC_SIZE)(%rax), %VMM(1) + VPTESTN %VMM(1), %VMM(1), %k0 KMOV %k0, %VRSI + /* Shift out zero CHAR matches that are before the beginning of + src (rdi). */ # ifdef USE_AS_WCSRCHR movl %edi, %ecx - and $(VEC_SIZE - 1), %ecx + andl $(VEC_SIZE - 1), %ecx shrl $2, %ecx # endif - shlx %SHIFT_REG, %VRDX, %VRDX - -# ifdef USE_AS_WCSRCHR - kmovw %edx, %k1 -# else - KMOV %VRDX, %k1 -# endif - - VPCOMPRESS %VMM(6), %VMM(1){%k1}{z} - /* We could technically just jmp back after the vpcompress but - it doesn't save any 16-byte blocks. */ shrx %SHIFT_REG, %VRSI, %VRSI + test %VRSI, %VRSI - jnz L(page_cross_return) - jmp L(page_cross_continue) - /* 1-byte from cache line. */ + jz L(page_cross_continue) + + /* Found zero CHAR so need to test for search CHAR. */ + VPCMP $0, %VMATCH, %VMM(1), %k1 + KMOV %k1, %VRAX + /* Shift out search CHAR matches that are before the beginning of + src (rdi). */ + shrx %SHIFT_REG, %VRAX, %VRAX + /* Check if any search CHAR match in range. */ + blsmsk %VRSI, %VRSI + and %VRSI, %VRAX + jz L(ret2) + bsr %VRAX, %VRAX +# ifdef USE_AS_WCSRCHR + leaq (%rdi, %rax, CHAR_SIZE), %rax +# else + addq %rdi, %rax +# endif +L(ret2): + ret + /* 3 bytes from cache-line for evex. + Crosses cache line for evex512. */ END(STRRCHR) #endif