From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 178E13858D28 for ; Tue, 20 Sep 2022 11:43:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 178E13858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663674227; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VZwwQpy0fvLcjbXW4bth1cJR8o+zv0VzWegtOos6tV0=; b=iQanw21yNejVZ8PwpUEvNzLmNZpfEDaztzOgk6RiNrCoRePOOtE3TmJVDhSSyLeTKgP1hE /29LKx6JBOCWkqdfkHi1xYtoNwOdjmL2x7ZRgO57dj3HqIATQ91gTTAtA3mEA5yNq3bvOj G5sHHiK77XAy1wfy9ieO/khh6buftWA= Received: from mail-il1-f197.google.com (mail-il1-f197.google.com [209.85.166.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-125-viOYH-lJOQeXee0Gg9pT2g-1; Tue, 20 Sep 2022 07:43:46 -0400 X-MC-Unique: viOYH-lJOQeXee0Gg9pT2g-1 Received: by mail-il1-f197.google.com with SMTP id i13-20020a056e02152d00b002f58aea654fso1407276ilu.20 for ; Tue, 20 Sep 2022 04:43:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=VZwwQpy0fvLcjbXW4bth1cJR8o+zv0VzWegtOos6tV0=; b=YeHPgopZ/C9ro0VJcsWs4jgaWw9cfFZGY0GFj2QKoTS7LNSwYv3eJnVATbj439xQpe O0BUXIIBK1hJwBFondvwxzPNVIdF1dVDH2JRSOsPjOF5oASqiEm/qRfDvHwq+UdF/H/i 2CfjFNe5xcqbVR6vgAvcRZAIjQsM73Ciy7oFDDPDhcFbRKdWGkiwEinPSfX4AjdJDZcm 0s54qzQAThShGBdUsx6o0aT5Om1vBP3Zh+WbPY1a9ib7qFvJykRHmHJUMZeU+rneWCPj DW5qMfPN9UREZENGxI9x0OU4XXutSpigYCNEmbv/pbwU4HhFi1zzHSWA2klN+gQ/ha+d ZClA== X-Gm-Message-State: ACrzQf37kEW3YVu87N3jX2W1QgNv+ishmxWlHp+MwUHAnRCdV8pbX2aG PwUN5ZaVyHxnfTxNNiJM6mAZoK1ZBmUx5yFMTlNGfj3v0rEUuOiRPXikIdqcbAwnY80/IfR2lCr skV2pVvrD4cq+EXV71rz4 X-Received: by 2002:a6b:b28b:0:b0:6a1:eb9e:bf62 with SMTP id b133-20020a6bb28b000000b006a1eb9ebf62mr8665939iof.87.1663674225634; Tue, 20 Sep 2022 04:43:45 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5L9N0fTiIza8LQFu60wnS5cYK19lHWqdO2F3aQJxsZFuQeTxU8vkFm4qbF0Dgh/i2/ZkrFPw== X-Received: by 2002:a6b:b28b:0:b0:6a1:eb9e:bf62 with SMTP id b133-20020a6bb28b000000b006a1eb9ebf62mr8665934iof.87.1663674225406; Tue, 20 Sep 2022 04:43:45 -0700 (PDT) Received: from fedora ([104.129.159.227]) by smtp.gmail.com with ESMTPSA id y1-20020a920901000000b002eb13760b7asm568549ilg.49.2022.09.20.04.43.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Sep 2022 04:43:44 -0700 (PDT) Date: Tue, 20 Sep 2022 07:43:41 -0400 From: Carlos O'Donell To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Subject: Re: [PATCH v5 03/17] Add string-maskoff.h generic header Message-ID: References: <20220919195920.956393-1-adhemerval.zanella@linaro.org> <20220919195920.956393-4-adhemerval.zanella@linaro.org> MIME-Version: 1.0 In-Reply-To: <20220919195920.956393-4-adhemerval.zanella@linaro.org> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Sep 19, 2022 at 04:59:06PM -0300, Adhemerval Zanella via Libc-alpha wrote: > Macros to operate on unaligned access for string operations: > > - create_mask: create a mask based on pointer alignment to sets up > non-zero bytes before the beginning of the word so a following > operation (such as find zero) might ignore these bytes. > > - highbit_mask: create a mask with high bit of each byte being 1, > and the low 7 bits being all the opposite of the input. I really appreciate the effort you've put into documenting the purpose of each function! It's really awesome to reach such nice coments. Thank you for that. I've gone through this to review the implementation and the descriptions. I think it needs a little more tweaking. > These macros are meant to be used on optimized vectorized string > implementations. > --- > sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++ > 1 file changed, 73 insertions(+) > create mode 100644 sysdeps/generic/string-maskoff.h > > diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h > new file mode 100644 > index 0000000000..831647bda6 > --- /dev/null > +++ b/sysdeps/generic/string-maskoff.h > @@ -0,0 +1,73 @@ > +/* Mask off bits. Generic C version. > + Copyright (C) 2022 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#ifndef _STRING_MASKOFF_H > +#define _STRING_MASKOFF_H 1 > + > +#include > +#include > +#include > +#include > + > +/* Provide a mask based on the pointer alignment that sets up non-zero > + bytes before the beginning of the word. It is used to mask off > + undesirable bits from an aligned read from an unaligned pointer. > + For instance, on a 64 bits machine with a pointer alignment of s/bits/-bit/g While it is technically correct English to say "A 64-bits machine", this is not the normative usage. I suggest we use the normative "64-bit machine." We can talk about 64 bits, and alignment as bits etc. > + 3 the function returns 0x0000000000ffffff for LE and 0xffffff0000000000 > + (meaning to mask off the initial 3 bytes). */ Missing "for BE" ? > +static inline op_t > +create_mask (uintptr_t i) > +{ > + i = i % sizeof (op_t); OK. Wrap the value. > + if (__BYTE_ORDER == __LITTLE_ENDIAN) > + return ~(((op_t)-1) << (i * CHAR_BIT)); > + else > + return ~(((op_t)-1) >> (i * CHAR_BIT)); OK. Shift. > +} > + > +/* Setup an word with each byte being c_in. For instance, on a 64 bits s/an/a/g s/bits/-bit/g > + machine with input as 0xce the functions returns 0xcececececececece. */ > +static inline op_t > +repeat_bytes (unsigned char c_in) > +{ > + return ((op_t)-1 / 0xff) * c_in; > +} How does the compiler do here on the various architectures to produce the deposit/expand instructions that could be used for this operation? aarch64 gcc trunk: ldrb r3, [r7, #7] @ zero_extendqisi2 mov r2, #16843009 mul r3, r2, r3 x86_64 gcc trunk: movzx eax, BYTE PTR [rbp-4] imul eax, eax, 16843009 s390x gcc12: ic %r1,167(%r11) lhi %r2,255 nr %r1,%r2 ms %r1,.L4-.L3(%r5) llgfr %r1,%r1 Looks OK, and the static inline will get optimized with the rest of the operations. > + > +/* Based on mask created by 'create_mask', mask off the high bit of each s/on/on a/g > + byte in the mask. It is used to mask off undesirable bits from an > + aligned read from an unaligned pointer, and also taking care to avoid s/and/while/g > + match possible bytes meant to be matched. For instance, on a 64 bits Suggest: matching possible bytes not meant to be matched. s/bits/-bits/g > + machine with a mask created from a pointer with an alignment of 3 > + (0x0000000000ffffff) the function returns 0x7f7f7f0000000000 for BE > + and 0x00000000007f7f7f for LE. */ > +static inline op_t > +highbit_mask (op_t m) > +{ > + return m & repeat_bytes (0x7f); OK. > +} > + > +/* Return the address of the op_t word containing the address P. For > + instance on address 0x0011223344556677 and op_t with size of 8, > + it returns 0x0011223344556670. */ Could you expand on this a bit more? It's a bit opaque what we might use this for (I have some ideas). > +static inline op_t * > +word_containing (char const *p) > +{ > + return (op_t *) (p - (uintptr_t) p % sizeof (op_t)); > +} > + > +#endif /* _STRING_MASKOFF_H */ > -- > 2.34.1 >