[PATCH 1/1] usb: avoid -Werror=address-of-packed-member

[Marek Vasut]

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 ?