From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pali =?utf-8?B?Um9ow6Fy?= Date: Sun, 7 Mar 2021 13:31:11 +0100 Subject: [PATCH u-boot 14/39] lib: crc32: make the crc_table variable non-const In-Reply-To: <20210307132636.69a4b753@nic.cz> References: <20210307042538.21229-1-marek.behun@nic.cz> <20210307042538.21229-15-marek.behun@nic.cz> <042b7e3b-511a-c822-dfbd-fe66aa5faa55@denx.de> <20210307055800.3fa65244@nic.cz> <20210307132636.69a4b753@nic.cz> Message-ID: <20210307123111.dygplcgck24xzqwu@pali> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sunday 07 March 2021 13:26:36 Marek Behun wrote: > On Sun, 7 Mar 2021 06:02:16 +0100 > Marek Vasut wrote: > > > On 3/7/21 5:58 AM, Marek Behun wrote: > > > On Sun, 7 Mar 2021 05:46:24 +0100 > > > Marek Vasut wrote: > > > > > >> On 3/7/21 5:25 AM, Marek Beh?n wrote: > > >>> When compiling with LTO, the compiler fails with an error saying that > > >>> `crc_table` causes a section type conflict with `efi_var_buf`. > > >>> > > >>> This is because both are declared to be in the same section (via macro > > >>> `__efi_runtime_data`), but one is const while the other is not. > > >>> > > >>> Make this variable non-const in order to fix this. > > >> > > >> This does not look right, the crc32 array is constant. > > >> Maybe what you want to do instead if create some __efi_constant_data > > >> section ? > > > > > > Yes, this was the easier solution, and maybe is not ideal. > > > > > > I thought it would not be much of a problem since this array can be > > > nonconstant (generated after boot) if CONFIG_DYNAMIC_CRC_TABLE is > > > enabled. > > > > > > Anyway I don't much understand the EFI code so I wanted to poke into it > > > as little as possible. > > > > Isn't the compiler capable of better optimization on constant stuff ? > > That's pretty much what prompted my suggestion to add separate section. > > Yes, but > - for this case I don't think the compiler actually can do any > significat optimizations when declaring the table const, since it has > to access > tab[(crc ^ (x)) & 255] > I tried with arm compiler just now, with -O2 and -Os, and it > generated the same code either way > - when using LTO, the compiler theoretically should be able to deduce > that the table is never written to > > But if people insist on declaring the table const, I will look into > this... If you try to overwrite a const variable from the same code unit then compiler throw an error. So declaring read-only variable as a const could prevent people to unintentionally overwrite read-only variable. And detect possible bad code.