From patchwork Sat Nov 11 03:20:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Noah Goldstein X-Patchwork-Id: 79609 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 27AAB3858031 for ; Sat, 11 Nov 2023 03:21:08 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x830.google.com (mail-qt1-x830.google.com [IPv6:2607:f8b0:4864:20::830]) by sourceware.org (Postfix) with ESMTPS id 598C13858D1E for ; Sat, 11 Nov 2023 03:20:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 598C13858D1E 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 598C13858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::830 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699672854; cv=none; b=WkyHahIOvNa+wFX0Ji2SZE2cNfCa7TADzrtVsve1dU8LQIZeuHW36AxKGoO2tvU8Mhcno9To687jhkDa1Sc+BCHyIXxjiymqHboYSJV1pSIcpA7Wctp6st8TTHVJD9iQ2u+/eih7YVWAAHHdCDEys//GZ5uj2+ONvInRhLp+ikI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699672854; c=relaxed/simple; bh=WVCRAwduq+UUrnTe3bRIYGOuVzbwe+kNnNmNGZBlD9c=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=Tib91L/rj25oUKLb9Ws45JXRT1QisaNEATqoc7vCtTS/LzmIHr9gYTPj52UedjVKCsC3YPTYQfUB6JX9FdM/CpNsHdlTd4ACCkOlnstfIEYKhoT3z7vTbF1sFnz3lhjBMeOoOEloL0bQs1CALIoeqcm7lrEkZw0qm+0sbWD12cg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-qt1-x830.google.com with SMTP id d75a77b69052e-41cd4446cf5so17410081cf.3 for ; Fri, 10 Nov 2023 19:20:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699672852; x=1700277652; 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=GNngv6HPFR6lkEarlSAUJ7TslJI6SLMwJMQebZVhZns=; b=egCCUDOCjarJr8qccjjiT9fk0A8n7zOdZ638eQ0eZfFrwE1A+4kd4AGs47C0q3eVYt MBDFxKkHwZS86VU5wFb8/clPy3YjaNbAVa0D9W56hFjmF589XffJDT5l2wrmFuZWF0HV HPU3qSV37bIzvSEFcCgerJEwinl8SD4bYDk019LDHJHmfqLxHbZrtFADB8Qmz4XaYGiv ehb+BVdFx/gINUaA/SgAfyoGqNk5vD43DOi4SfZCHpJ8hfVz1mxr6VMQBmW6nZUYD3/T eewx0yFQPmu+Kys7Vf+0uLh5liARRz6AAQZAUNTtcUrW38odutPiWjt3b2l3jmqA9Dh1 I4gQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699672852; x=1700277652; 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=GNngv6HPFR6lkEarlSAUJ7TslJI6SLMwJMQebZVhZns=; b=VyMpAMn/c+72UjjBwMe/QzlGYkPcckvFJkz7HfdXFU65qcKLe9wVfHBQ3tbwbs0+dn WqDw3B7lvBwoRu6SVo+KOHwXdea9KZCUr3gBNyvHtnKd9eAda1Ptz2RGRaiBlAZup6rS SYUmBt9Y49FRBn17wlrFrPaUuoYDzRAWYjRujl94PQ0T925ii3sBKGMLHC2ppG0HFALJ kIFp9zy/6Mjuw366ForGKcrpG6paZFReyYQkrO7yVb9SM5I8EMHj2xktV6BsYJ9uJXSy zfus9hD4Yk6ZbOvhzlThL7MtT95Vc3R1M8ZGquRlvpIfnjffxeg1F7JuSALhPf9RlHoQ fNMw== X-Gm-Message-State: AOJu0Yx9rtfkGiBRz6lUp90M2iNaxWRgzqqiUp/5jvXgZpyQRmnQft30 5ADyuLHZ+FfIAmzYUQQd1XjgZKYi0lA= X-Google-Smtp-Source: AGHT+IGLjRtTpkZCq3E9KOgzP0Sf56+8YyyCNqTmqSLUDFNgrISUakzZmpZqKfZPqzOfpHXsCeTqUw== X-Received: by 2002:ac8:5cd0:0:b0:41e:1754:daa3 with SMTP id s16-20020ac85cd0000000b0041e1754daa3mr1311343qta.29.1699672852051; Fri, 10 Nov 2023 19:20:52 -0800 (PST) Received: from noahgold-desk.intel.com ([192.55.54.48]) by smtp.gmail.com with ESMTPSA id t23-20020ac87617000000b00418189b689csm282219qtq.10.2023.11.10.19.20.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Nov 2023 19:20:51 -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 v4] x86: Fix unchecked AVX512-VBMI2 usage in strrchr-evex-base.S Date: Fri, 10 Nov 2023 21:20:47 -0600 Message-Id: <20231111032047.2138614-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+patchwork=sourceware.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 | 60 ++++++++++++-------- 1 file changed, 36 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..9cab21c09c 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,7 +77,7 @@ 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) @@ -167,7 +165,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 +337,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