From patchwork Wed Nov 8 21:33:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Noah Goldstein X-Patchwork-Id: 1861767 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=jybKwkLL; 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 4SQdcj6MZJz1yQl for ; Thu, 9 Nov 2023 08:33:41 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EAF543864844 for ; Wed, 8 Nov 2023 21:33:39 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-ej1-x62b.google.com (mail-ej1-x62b.google.com [IPv6:2a00:1450:4864:20::62b]) by sourceware.org (Postfix) with ESMTPS id BFB383864844 for ; Wed, 8 Nov 2023 21:33:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BFB383864844 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 BFB383864844 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::62b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699479211; cv=none; b=s0U3hkYq/m0e/r/oD351lSF6FCOEAbGXFZopH6fgLh5hbHIR+XzdtW8GoBRI9ByW1hIe1yXYvA24fKhXGiyzufwvC8qc1jE/Fao5D7QceQSzc4F5A2ZxIPmzFMH40hYiGhJTFE1oGu+3z4SggHYhV7xC8gYBgpaX/e01czIIOtY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699479211; c=relaxed/simple; bh=dceORbmwm/biJpnTF3Axc3SlbiKnkszQW7oxsUpTNCE=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=gPN9bhFDojJ5i0M2g+kRAEcXvBK7w9DJ+xI8LnGY+ifiHTIj4FsfaBc1yqe58bUJuoQPU7WznuB9bpyC34Kbxj4Erx6RQ7a3BQ77AcgtqifEIRHgPzPyODOR3ufeF8OzHre1q+D17Y98NFqPuHt0GbRYsZAX2iOKKZ3mcf/osZ4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ej1-x62b.google.com with SMTP id a640c23a62f3a-9de7a43bd1aso30299966b.3 for ; Wed, 08 Nov 2023 13:33:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699479207; x=1700084007; 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=k9LkI6zlSXm2dG9fjZ7Qwd4MP+FNT7Kx7qcF1z0d6FU=; b=jybKwkLL6rr9NVPjcXxDwkcdLZhhbFs8J2yDE1/wTxGnlF0/bNCv+Dc1aexTMFA+PI p+FVPS/77ghnH3pRBbDhnpnr5K160HnLhQoUFegLjpndGVDwaKeMs2DeINUuBpjP0MOW I0CDgsd88IKo2c5CGsITH+8lLvTPlAeBNk3bqhbD0ZrT9liUHafVn0gqXEWguGBRPp0f hlY5F8NfzQhgfD4b3ANjudvdpw08+0MLNGmMTVr9dP2+r1vQZEIXCSAv4uemE51LGpwL o0BKMf1Md2e8dAfNJGn81/Um3gXL01VymZ+ytCXJvt5A02Gp3sekDlIv5+CJPq8furbF 4Zpg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699479207; x=1700084007; 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=k9LkI6zlSXm2dG9fjZ7Qwd4MP+FNT7Kx7qcF1z0d6FU=; b=pgg6DxqkiIE6BBUcKTsFp9rtCmE0XVRUbPhhcEAsWsh5QGjlZdVVUaWDaFMta9Iija ZgiVhpyH8XV9YJefr+wukSmOzZFNsE9fdu+vDttq6JhudAs7jnqeiOSnmWtBeYgN49AM laSXPGrSYYpH3vrWfjhr3wWNBy4JXbi+iwkTdYjqLYV49FQMCkGCteWpdfXpY2e6wNTn ZuzgFTgU1/OS56S14UUZj9S3Ksxx8Y5OSzyYrLDmgXssXQn2o3Lc5eOj5pIXf8Gm7M/y 1DqKqGDDaoVBZ2rzD4a35lC6fcLvAkZdJNIl5QZgQj2sdPLTwHecu7GjP3DGMvEmTNwu jdmg== X-Gm-Message-State: AOJu0YxYGE/1y+rHYmeVJAppmFdpEzHaidX5YD6yEKxRJqvgk6+znNAD fxVqp9HteCBP6BbdN8NYKggCnVQ3w3U= X-Google-Smtp-Source: AGHT+IG/U3Uus8IxpYGMllhiJuteLBbYj7KxpRdg7Wln4xAj59rWIO4Pk0wrdi9bYc8VyeDNp0caNw== X-Received: by 2002:a17:907:3201:b0:9bf:f20:876d with SMTP id xg1-20020a170907320100b009bf0f20876dmr2358234ejb.75.1699479206653; Wed, 08 Nov 2023 13:33:26 -0800 (PST) Received: from noahgold-desk.intel.com ([192.55.54.48]) by smtp.gmail.com with ESMTPSA id jp27-20020a170906f75b00b0099290e2c163sm1595615ejb.204.2023.11.08.13.33.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Nov 2023 13:33:26 -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 v2] x86: Fix unchecked AVX512-VBMI2 usage in strrchr-evex-base.S Date: Wed, 8 Nov 2023 15:33:19 -0600 Message-Id: <20231108213319.1526816-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 | 66 +++++++++++++------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/sysdeps/x86_64/multiarch/strrchr-evex-base.S b/sysdeps/x86_64/multiarch/strrchr-evex-base.S index cd6a0a870a..7fb0a6a543 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,56 @@ 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 + shrx %SHIFT_REG, %VRSI, %VRSI -# ifdef USE_AS_WCSRCHR - kmovw %edx, %k1 + test %VRSI, %VRSI + 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 + /* For strrchr VEC_SIZE == 64 we are just at the end of a cacheline + here so to save code-size just re-use return logic for first + VEC. This is relatively cold code (page cross). */ +# if VEC_SIZE == 64 && !(defined USE_AS_WCSRCHR) + jmp L(page_cross_return) + /* 6 bytes from cache-line. */ # else - KMOV %VRDX, %k1 + /* 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. */ # 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. */ END(STRRCHR) #endif