From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x1134.google.com (mail-yw1-x1134.google.com [IPv6:2607:f8b0:4864:20::1134]) by sourceware.org (Postfix) with ESMTPS id C2B3C394D883 for ; Thu, 19 May 2022 14:50:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C2B3C394D883 Received: by mail-yw1-x1134.google.com with SMTP id 00721157ae682-2ff53d86abbso40100407b3.8 for ; Thu, 19 May 2022 07:50:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=M2sfD+wDWvSgdMRQqLJ/ZaB3F/lfCKchzEd6NGRbfnw=; b=quPvF2cdde7j9KroWqaATISjzludAebky92MsmT/a+44PNiqpJVQS04OKr7Ii+BXXC Q4EDR0vDQ6I13zWiO3Mnoyn0Rujd3afzA4j5n3lL010e5P24Rj+uaKW0aUDjrLAzS7HG szTc8shF8r0BFGSm9y12rGvQ3XEGGhvLFyx1iHdsLQoBmxL5e8GtFz6040fdyBgljW8a ASR7VN4V52Px9VtS7ncsmqJwpmvpRtD6IeWEpmTIBR+6dZ926jtCcuHb3Y/3pkXvKhoA 64EAUWVIVkZNajUi0PnrzrgTJJYosaNEmeUMKzTSu/YTWuVOW17H0AIMrEdqnVQzl9nU lddw== X-Gm-Message-State: AOAM532rwZzzIFgbSul5JTY0i4awVTmT/7MQ39QFHuJ7jeRojcBbgjWr VRV/21RZOA0QWxyS3x/uPqtXrluomuQ8kmguhWMsamgu X-Google-Smtp-Source: ABdhPJxOCEj+kQrYH8bB8X2+myAdEZZ6wW3fjQqC/h0IwbGQvuIK2DiZ+QfbH9UbBx+sItAuouwKzcU3mkAvxgk0q4o= X-Received: by 2002:a81:c44b:0:b0:2d6:4726:ef4b with SMTP id s11-20020a81c44b000000b002d64726ef4bmr5018920ywj.184.1652971829098; Thu, 19 May 2022 07:50:29 -0700 (PDT) MIME-Version: 1.0 References: <20220414041231.926415-1-goldstein.w.n@gmail.com> <20220518172635.291119-1-goldstein.w.n@gmail.com> In-Reply-To: From: Noah Goldstein Date: Thu, 19 May 2022 09:50:18 -0500 Message-ID: Subject: Re: [PATCH v10 1/6] elf: Refactor dl_new_hash so it can be tested / benchmarked To: Siddhesh Poyarekar Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, 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.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 May 2022 14:50:32 -0000 On Thu, May 19, 2022 at 9:47 AM Siddhesh Poyarekar wrote: > > On 18/05/2022 22:56, Noah Goldstein via Libc-alpha wrote: > > No change to the code other than moving the function to > > dl-new-hash.h. Changed name so its now in the reserved namespace. > > --- > > elf/dl-lookup.c | 13 ++----------- > > elf/dl-new-hash.h | 40 ++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 42 insertions(+), 11 deletions(-) > > create mode 100644 elf/dl-new-hash.h > > > > diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c > > index 989b073e4f..a42f6d5390 100644 > > --- a/elf/dl-lookup.c > > +++ b/elf/dl-lookup.c > > @@ -24,6 +24,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -558,16 +559,6 @@ skip: > > } > > > > > > -static uint32_t > > -dl_new_hash (const char *s) > > -{ > > - uint32_t h = 5381; > > - for (unsigned char c = *s; c != '\0'; c = *++s) > > - h = h * 33 + c; > > - return h; > > -} > > - > > - > > /* Add extra dependency on MAP to UNDEF_MAP. */ > > static int > > add_dependency (struct link_map *undef_map, struct link_map *map, int flags) > > @@ -816,7 +807,7 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map, > > const struct r_found_version *version, > > int type_class, int flags, struct link_map *skip_map) > > { > > - const unsigned int new_hash = dl_new_hash (undef_name); > > + const unsigned int new_hash = _dl_new_hash (undef_name); > > unsigned long int old_hash = 0xffffffff; > > struct sym_val current_value = { NULL, NULL }; > > struct r_scope_elem **scope = symbol_scope; > > diff --git a/elf/dl-new-hash.h b/elf/dl-new-hash.h > > new file mode 100644 > > index 0000000000..8641bb4196 > > --- /dev/null > > +++ b/elf/dl-new-hash.h > > @@ -0,0 +1,40 @@ > > +/* _dl_new_hash for elf symbol lookup > > + 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 _DL_NEW_HASH_H > > +#define _DL_NEW_HASH_H 1 > > + > > +#include > > +/* For __always_inline. */ > > +#include > > + > > +static __always_inline uint32_t > > +__attribute__ ((unused)) > > +_dl_new_hash (const char *s) > > +{ > > + uint32_t h = 5381; > > + for (unsigned char c = *s; c != '\0'; c = *++s) > > + h = h * 33 + c; > > + return h; > > +} > > + > > +/* For testing/benchmarking purposes. */ > > +#define __simple_dl_new_hash _dl_new_hash > > + > > + > > +#endif /* dl-new-hash.h */ > > Uhmm, you're going to call it __simple_dl_new_hash. A bit roundabout, > but OK. It's in a header. Doesn't it need to be in reserved namespace? > > Reviewed-by: Siddhesh Poyarekar