On Tue, Dec 17, 2019 at 08:52:01PM +0100, Marek Vasut wrote:
On 12/17/19 8:32 PM, Heinrich Schuchardt wrote:
On 12/17/19 1:19 PM, Marek Vasut wrote:
On 12/17/19 12:59 PM, Heinrich Schuchardt wrote:
On 12/17/19 12:19 PM, Marek Vasut wrote:
On 12/17/19 12:14 PM, Heinrich Schuchardt wrote:
On 12/17/19 11:00 AM, Marek Vasut wrote: > On 12/17/19 10:27 AM, Heinrich Schuchardt wrote: >> With GCC 9.2.1 errors of type -Werror=address-of-packed-member occur >> when >> passing a member of a packed structure to le16_to_cpus() on a big >> endian >> system (e.g. P2041RDB_defconfig). >> >> Replace le16_to_cpus() by get_unaligned_le16(). Check >> defined(__BIG_ENDIAN) >> to avoid the introduction of unnecessary instructions on little >> endian >> systems as seen on aarch64. > > I would expect the compiler would optimize such stuff out ? > Can we do without the ifdef ?
When compiling qemu_arm64_defconfig without the #ifdef the GCC 9.2.1 adds assembler instructions:
Why ?
I am not a GCC developer. I simply observed that GCC currently cannot optimize this away on its own. That is why I added the #ifdef.
Are we now adding workarounds instead of solving issues properly?
Identifying that bit operations like << 8 in __get_unaligned_le16() have zero effect is not an easy task when developing a compiler. You would have to follow the flow of every bit.
Maybe the fix is then to somehow optimize the get_unaligned_le16() to help the compiler ?
Inside get_unaligned_le16() it is not known that we will be reassigning to the same memory location. So I cannot imagine what to improve here.
You could invent new functions for in place byte swapping. But that would only move the #ifdef to a different place.
Isn't there already such a function in Linux ?
Also, why would you need the ifdef if the compiler would now know that the operation is noop ?
This particular patch looks to me exactly why we want to follow the Linux Kernel and disable this particular warning for GCC like we already do for LLVM.