Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752678AbdIVVfC (ORCPT ); Fri, 22 Sep 2017 17:35:02 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:59935 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752421AbdIVVe7 (ORCPT ); Fri, 22 Sep 2017 17:34:59 -0400 Subject: Re: [PATCH] lib: zstd: make const array rtbTable static, reduces object code size To: Nick Terrell , Chris Mason References: <20170922145946.14316-1-colin.king@canonical.com> <6C490CCD-32E1-4AC3-BCCF-5EB3A56EA887@fb.com> Cc: "kernel-janitors@vger.kernel.org" , "linux-kernel@vger.kernel.org" From: Colin Ian King Message-ID: <2c51727b-8adf-add9-5292-a267cfec3a04@canonical.com> Date: Fri, 22 Sep 2017 22:34:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <6C490CCD-32E1-4AC3-BCCF-5EB3A56EA887@fb.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2429 Lines: 56 On 22/09/17 20:14, Nick Terrell wrote: > On 9/22/17, 8:00 AM, "linux-kernel-owner@vger.kernel.org on behalf of Colin King" wrote: >> From: Colin Ian King >> >> Don't populate const array rtbTable on the stack, instead make it >> static. Also split overly long line to clean a chechkpach warning. >> Makes the object code smaller by nearly 500 bytes: >> >> Before: >> text data bss dec hex filename >> 13297 104 0 13401 3459 lib/zstd/fse_compress.o >> >> After: >> text data bss dec hex filename >> 12742 160 0 12902 3266 lib/zstd/fse_compress.o >> >> (gcc 6.3.0, x86-64) >> >> Signed-off-by: Colin Ian King > > I tested your patch with gcc-7.1 on x86, and benchmarked the speed on > upstream zstd. There isn't a noticeable speed difference, since it isn't a > particularly hot piece of code. Would you be able to submit the same patch > upstream [1], or would you be okay with me porting it back upstream, so it > doesn't get lost on an update? Since you have more contact with the zstd codebase it may be preferable for me to pass the port back to upstream over to you Nick (if that's OK). As it stands, I did a quick check by building the original zstd codebase with gcc 7.2 and didn't observe any object code size changes between the original and the patched code. I see that it's optimized with -O3 and its not using the same gcc flags as the kernel, so I suspect that may need exploring further to see why there is such a large change on the kernel version and that of the native user space library build. > > I didn't expect gcc to leave constant arrays on the stack, that seems > silly. Clang makes it static, but gcc loads it onto the stack, and in 6.3+ > it saves the data statically, and then uses vector instructions to load it > onto the stack [2]. Oh, that latter information is interesting, didn't know that. Thanks for testing. Colin > > Tested-by: Nick Terrell > > [1] https://github.com/facebook/zstd > [2] https://godbolt.org/g/fvTcED > > > N�����r��y���b�X��ǧv�^�)޺{.n�+���z�ޖ6���+�)���w*jg��������ݢj/���z�ޖ��2�ޙ���&�)ߡ�a�����G���h��j:+v���w�٥ >