On Tue, Dec 31, 2019 at 12:41 AM Tom Rini <tr...@konsulko.com> wrote: > > 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.
Agree. I think we can follow Linux commit 6f303d60534c46aa1a239f29c321f95c83dda748 -- Best Regards Masahiro Yamada