From patchwork Tue Dec 8 08:44:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxim Uvarov X-Patchwork-Id: 57837 Delivered-To: patch@linaro.org Received: by 10.112.147.194 with SMTP id tm2csp1658120lbb; Tue, 8 Dec 2015 00:44:30 -0800 (PST) X-Received: by 10.55.22.29 with SMTP id g29mr2882688qkh.100.1449564270884; Tue, 08 Dec 2015 00:44:30 -0800 (PST) Return-Path: Received: from lists.linaro.org (lists.linaro.org. [54.225.227.206]) by mx.google.com with ESMTP id w15si878703qhw.84.2015.12.08.00.44.30; Tue, 08 Dec 2015 00:44:30 -0800 (PST) Received-SPF: pass (google.com: domain of lng-odp-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; Authentication-Results: mx.google.com; spf=pass (google.com: domain of lng-odp-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) smtp.mailfrom=lng-odp-bounces@lists.linaro.org; dkim=neutral (body hash did not verify) header.i=@linaro-org.20150623.gappssmtp.com Received: by lists.linaro.org (Postfix, from userid 109) id 2DD8D61CE1; Tue, 8 Dec 2015 08:44:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on ip-10-142-244-252 X-Spam-Level: X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, T_DKIM_INVALID, URIBL_BLOCKED autolearn=disabled version=3.4.0 Received: from [127.0.0.1] (localhost [127.0.0.1]) by lists.linaro.org (Postfix) with ESMTP id C665C61CC8; Tue, 8 Dec 2015 08:44:23 +0000 (UTC) X-Original-To: lng-odp@lists.linaro.org Delivered-To: lng-odp@lists.linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id AECA961CC9; Tue, 8 Dec 2015 08:44:21 +0000 (UTC) Received: from mail-lf0-f54.google.com (mail-lf0-f54.google.com [209.85.215.54]) by lists.linaro.org (Postfix) with ESMTPS id 2B52C61CC5 for ; Tue, 8 Dec 2015 08:44:20 +0000 (UTC) Received: by lfdl133 with SMTP id l133so8025311lfd.2 for ; Tue, 08 Dec 2015 00:44:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro-org.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-type:content-transfer-encoding; bh=WwNbh6+K8hemuTkATlPZq9d+fTUrLGSfJvUojC/nnbc=; b=ajUwZMakiJftb7XDRPo3bTtJ3mBATSHdt2ouDodS4l8LXbImvRs7KJUCPB66Gms+ti 9cVElJRZRKKAHws9rkFIEtEZH6cBoWgxKeTxykugjHm85pSQgDGqAoyPJjYwM1OcO9fr HfNu+TxhdFXVTKS5ulVR7s1EA0Ule4TYMcQ487UNLjksljw0jmMORmAN44qbHPtzlZ2q 5AzHkqOUy8SYaq9wy6lfalscTKxC2KvSYxbf0AAxaHIThYt6uLoI96l5yHcmH/jAJjag vyLyT8sGsC3O3wV+pZ8D4u0rvRVCbOUhdsz/OgviKMfPM1WCXAdX99m6rjs+wkuSnfP/ XjRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=WwNbh6+K8hemuTkATlPZq9d+fTUrLGSfJvUojC/nnbc=; b=lCZtVss7W91RAZWQ1UCHRI5IP7O2Nuzo7Sikj/3qRC0NBXoC8tm2WJFIAMLaW+pqJn 4cMMXigFIz1kWN9j+Ku03fpYXwBIKG8rUHIrApDWx1HY3R8R9lBpjzYzeS+Qma95z/ys vVS7qeA0Zl5+Vj206fGeGsAf6v3Hqe6m7AjdefZ4ZE7sx2oA8ckIAvVERC8UbJvhT1/d tglDt4WVw6IdY8vh7ctgIs05N73CppODBLZBO+/Bq66jFN6yIeUhADyVbojpSIjvn/6G yEJ3mcqIe2A9MwhDBklnPnj6LyolLMZ+v4Xcu9k4ck8HSZCo3jvqxxG0NpB+156MkgLE SOYQ== X-Gm-Message-State: ALoCoQn5mkVF8wJk6E/vP6PGknmF5Mf2HT996Q4adcGJEUJrEWMtBGlayPMrf+AbPq+gDH1y1CMwlhGMFFP8s6oh7CUPGUskvw== X-Received: by 10.25.35.194 with SMTP id j185mr1078049lfj.62.1449564259006; Tue, 08 Dec 2015 00:44:19 -0800 (PST) Received: from [193.168.1.37] (ppp91-76-173-134.pppoe.mtu-net.ru. [91.76.173.134]) by smtp.googlemail.com with ESMTPSA id i7sm368697lbo.39.2015.12.08.00.44.17 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 08 Dec 2015 00:44:18 -0800 (PST) To: "Savolainen, Petri (Nokia - FI/Espoo)" , "lng-odp@lists.linaro.org" References: <1449486553-16699-1-git-send-email-bill.fischofer@linaro.org> <56659F2C.30303@linaro.org> From: Maxim Uvarov Message-ID: <56669862.9000509@linaro.org> Date: Tue, 8 Dec 2015 11:44:18 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: X-Topics: Architecture patch Subject: Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: tm: use odp_hash_crc32c() api to avoid arch issues X-BeenThere: lng-odp@lists.linaro.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: "The OpenDataPlane \(ODP\) List" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: lng-odp-bounces@lists.linaro.org Sender: "lng-odp" On 12/08/2015 10:50, Savolainen, Petri (Nokia - FI/Espoo) wrote: > Bill, > > hash_tbl_entry is now uintptr_t, which is 64 bits on a 64 bit build and 32 bits on a 32 bit build. These shifts and all other lines that expect that hash_tbl_entry (pointer size) is 64 bits should be updated also. > > -Petri > Yes, that is what I said before. For now we can go with simple using 64 bit hash entries for 32 bit case also. Where 6 upper bits will be for defining first or secondary hash and it's size. And all lower 32 for address pointer. I.e. 26 bits will be lost. #define PRIMARY_HASH_TBL_SIZE (16 * 1024) 26 * PRIMARY_HASH_TBL_SIZE = 425984 = 53248 bytes = 13 pages; #define SECONDARY_HASH_TBL_SIZE 128 26 * SECONDARY_HASH_TBL_SIZE = 3328 = 416 bytes So we lost for 32 bits some memory but code has to work in the same way as for 64 bits. After that if memory efficient fix will be needed for somebody than he can fix it. Or we will reuse some odp hash table api instead of odp_name_table.c. So for now I would do quick fix with some comment that there is unused memory for 32 bit case. BR, Maxim. >> -----Original Message----- >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT >> Maxim Uvarov >> Sent: Monday, December 07, 2015 5:01 PM >> To: lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: tm: use >> odp_hash_crc32c() api to avoid arch issues >> >> shift still is not fixed: >> >> odp_name_table.c:186:4: error: right shift count >= width of type [- >> Werror] >> if ((hash_tbl_entry >> 48) == 0x7FFF) >> ^ >> odp_name_table.c:188:4: error: right shift count >= width of type [- >> Werror] >> else if ((hash_tbl_entry >> 48) == 0) >> >> >> On 12/07/2015 14:09, Bill Fischofer wrote: >>> Change the internal hash_name_and_kind() function to eliminate the use >>> of architecture-specific hash instructions. This eliminates build issues >>> surrounding ARM variants. Future optimizations will use the arch >> directory >>> consistent with other ODP modules. >>> >>> Signed-off-by: Bill Fischofer >>> --- >>> platform/linux-generic/odp_name_table.c | 184 +----------------------- >> -------- >>> 1 file changed, 2 insertions(+), 182 deletions(-) >>> >>> diff --git a/platform/linux-generic/odp_name_table.c b/platform/linux- >> generic/odp_name_table.c >>> index 10a760e..b4a9081 100644 >>> --- a/platform/linux-generic/odp_name_table.c >>> +++ b/platform/linux-generic/odp_name_table.c >>> @@ -48,140 +48,6 @@ >>> >>> #define SECONDARY_HASH_HISTO_PRINT 1 >>> >>> - /* #define USE_AES */ >>> - >>> -#if defined __x86_64__ || defined __i386__ >>> - >>> -#ifdef USE_AES >>> - >>> -typedef long long int v2di __attribute__((vector_size(16))); >>> - >>> -static const v2di HASH_CONST1 = { 0x123456, 0xFEBCDA383 }; >>> -static const v2di HASH_CONST2 = { 0x493BA3F689, 0x102F5D73A8C }; >>> - >>> -#define PLATFORM_HASH_STATE v2di >>> - >>> -#define PLATFORM_HASH32_INIT(hash_state, name_len) \ >>> - ({ \ >>> - hash_state = HASH_CONST1; \ >>> - hash_state[0] ^= name_len; \ >>> - }) >>> - >>> -#define PLATFORM_HASH32(hash_state, name_word) \ >>> - ({ \ >>> - v2di data; \ >>> - \ >>> - data[0] = name_word; \ >>> - data[1] = name_word << 1; \ >>> - hash_state = __builtin_ia32_aesenc128(hash_state, data); \ >>> - }) >>> - >>> -#define PLATFORM_HASH32_FINISH(hash_state, kind) >> \ >>> - ({ \ >>> - uint64_t result; \ >>> - v2di data; \ >>> - \ >>> - data[0] = name_kind; \ >>> - data[1] = name_kind << 7; \ >>> - hash_state = __builtin_ia32_aesenc128(hash_state, data); \ >>> - hash_state = __builtin_ia32_aesenc128(hash_state, \ >>> - HASH_CONST2); \ >>> - hash_state = __builtin_ia32_aesenc128(hash_state, \ >>> - HASH_CONST1); \ >>> - result = (uint64_t)hash_state[0] ^ hash_state[1]; \ >>> - result = result ^ result >> 32; \ >>> - (uint32_t)result; \ >>> - }) >>> - >>> -#else >>> - >>> -#define PLATFORM_HASH_STATE uint64_t >>> - >>> -#define PLATFORM_HASH32_INIT(hash_state, name_len) \ >>> - ({ \ >>> - hash_state = (uint64_t)name_len; \ >>> - hash_state |= hash_state << 8; \ >>> - hash_state |= hash_state << 16; \ >>> - hash_state |= hash_state << 32; \ >>> - }) >>> - >>> -#define PLATFORM_HASH32(hash_state, name_word) \ >>> - ({ \ >>> - uint64_t temp; \ >>> - \ >>> - temp = ((uint64_t)name_word) * 0xFEFDFCF5; \ >>> - hash_state = hash_state * 0xFF; \ >>> - hash_state ^= temp ^ (uint64_t)name_word; \ >>> - }) >>> - >>> -#define PLATFORM_HASH32_FINISH(hash_state, kind) \ >>> - ({ \ >>> - hash_state ^= (((uint32_t)kind) << 13); \ >>> - hash_state = hash_state * 0xFEFDFCF5; \ >>> - hash_state = hash_state ^ hash_state >> 32; \ >>> - hash_state = hash_state % 0xFEEDDCCBBAA1; \ >>> - hash_state = hash_state ^ hash_state >> 32; \ >>> - (uint32_t)hash_state; \ >>> - }) >>> - >>> -#endif >>> - >>> -#elif defined(__tile_gx__) >>> - >>> -#define PLATFORM_HASH_STATE uint32_t >>> - >>> -#define PLATFORM_HASH32_INIT(hash_state, name_len) \ >>> - ({ \ >>> - hash_state = 0xFEFDFCF5; \ >>> - hash_state = __insn_crc_32_32(hash_state, name_len); \ >>> - }) >>> - >>> -#define PLATFORM_HASH32(hash_state, name_word) \ >>> - ({ \ >>> - hash_state = __insn_crc_32_32(hash_state, name_word); \ >>> - }) >>> - >>> -#define PLATFORM_HASH32_FINISH(hash_state, kind) \ >>> - ({ \ >>> - hash_state = __insn_crc_32_32(hash_state, kind); \ >>> - hash_state = __insn_crc_32_32(hash_state, 0xFFFFFFFF); \ >>> - hash_state = __insn_crc_32_32(hash_state, 0xFEFDFCF5); \ >>> - (uint32_t)hash_state; \ >>> - }) >>> - >>> -#elif defined(__arm__) || defined(__aarch64__) >>> - >>> -#define PLATFORM_HASH_STATE uint32_t >>> - >>> -static inline uint32_t __crc32w(uint32_t crc, uint32_t value) >>> -{ >>> - __asm__("crc32w %w[c], %w[c], %w[v]":[c]"+r"(crc):[v]"r"(value)); >>> - return crc; >>> -} >>> - >>> -#define PLATFORM_HASH32_INIT(hash_state, name_len) \ >>> - ({ \ >>> - hash_state = 0xFEFDFCF5; \ >>> - hash_state = __crc32w(hash_state, name_len); \ >>> - }) >>> - >>> -#define PLATFORM_HASH32(hash_state, name_word) \ >>> - ({ \ >>> - __crc32w(hash_state, name_word); \ >>> - }) >>> - >>> -#define PLATFORM_HASH32_FINISH(hash_state, kind) \ >>> - ({ \ >>> - hash_state = __crc32w(hash_state, kind); \ >>> - hash_state = __crc32w(hash_state, 0xFFFFFFFF); \ >>> - hash_state = __crc32w(hash_state, 0xFEFDFCF5); \ >>> - (uint32_t)hash_state; \ >>> - }) >>> - >>> -#else >>> -#error "Need to define PLATFORM_DEPENDENT_HASH32 macro" >>> -#endif >>> - >>> typedef struct name_tbl_entry_s name_tbl_entry_t; >>> >>> /* It is important for most platforms that the following struct fit >> within >>> @@ -227,7 +93,7 @@ typedef struct { >>> * NOT zero then this values points to a (linked list of) >> name_table_entry_t >>> * records AND the bottom 6 bits are the length of this list. >>> */ >>> -typedef uint64_t hash_tbl_entry_t; >>> +typedef uintptr_t hash_tbl_entry_t; >>> >>> typedef struct { >>> hash_tbl_entry_t hash_entries[SECONDARY_HASH_TBL_SIZE]; >>> @@ -275,53 +141,7 @@ static void aligned_free(void *mem_ptr) >>> >>> static uint32_t hash_name_and_kind(const char *name, uint8_t >> name_kind) >>> { >>> - PLATFORM_HASH_STATE hash_state; >>> - uint32_t name_len, name_word, hash_value; >>> - uint32_t bytes[4]; >>> - >>> - name_len = strlen(name); >>> - PLATFORM_HASH32_INIT(hash_state, name_len); >>> - >>> - while (4 <= name_len) { >>> - /* Get the next four characters. Note that endianness doesn't >>> - * matter here! Also note that this assumes that there is >>> - * either no alignment loading restrictions OR that name is >>> - * 32-bit aligned. Shouldn't be too hard to add code to deal >>> - * with the case when this assumption is false. >>> - */ >>> - /* name_word = *((uint32_t *)name); */ >>> - bytes[0] = name[0]; >>> - bytes[1] = name[1]; >>> - bytes[2] = name[2]; >>> - bytes[3] = name[3]; >>> - name_word = (bytes[3] << 24) | (bytes[2] << 16) | >>> - (bytes[1] << 8) | bytes[0]; >>> - PLATFORM_HASH32(hash_state, name_word); >>> - >>> - name_len -= 4; >>> - name += 4; >>> - } >>> - >>> - if (name_len != 0) { >>> - name_word = 0; >>> - >>> - if (2 <= name_len) { >>> - /* name_word = (uint32_t)*((uint16_t *)name); */ >>> - bytes[0] = name[0]; >>> - bytes[1] = name[1]; >>> - name_word |= (bytes[1] << 8) | bytes[0]; >>> - name_len -= 2; >>> - name += 2; >>> - } >>> - >>> - if (name_len == 1) >>> - name_word |= ((uint32_t)*name) << 16; >>> - >>> - PLATFORM_HASH32(hash_state, name_word); >>> - } >>> - >>> - hash_value = PLATFORM_HASH32_FINISH(hash_state, name_kind); >>> - return hash_value; >>> + return odp_hash_crc32c(name, strlen(name), name_kind); >>> } >>> >>> static uint32_t linked_list_len(name_tbl_entry_t *name_tbl_entry) >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp --- a/platform/linux-generic/odp_name_table.c +++ b/platform/linux-generic/odp_name_table.c @@ -93,7 +93,7 @@ typedef struct { * NOT zero then this values points to a (linked list of) name_table_entry_t * records AND the bottom 6 bits are the length of this list. */ -typedef uintptr_t hash_tbl_entry_t; +typedef uint64_t hash_tbl_entry_t;