Re: [PATCH] drm/format-helper: fix build

[Matthew Auld <matthew.auld@intel.com>]

Hi,

On 02/04/2025 11:53, Thomas Zimmermann wrote:
> Hi
> 
> Am 02.04.25 um 12:44 schrieb Matthew Auld:
>> Build fails with:
>>
>> error: multiple unsequenced modifications to 'sbuf32'
>> [-Werror,-Wunsequenced]
>>    264 |                         le32_to_cpup(sbuf32++),
>>        |                                            ^
>>    265 |                         le32_to_cpup(sbuf32++),
>>        |                                            ~~
>>
>> With that move the increment of the sbuf32 pointer to the end of the
>> loop, instead of inside the array list initializer, where the
>> order/sequence of the sbuf32 pointer modifications is not defined.
>>
>> Fixes: 58523a25cbf7 ("drm/format-helper: Optimize 32-to-24-bpp 
>> conversion")
>> Fixes: 3f31a017ddbc ("drm/format-helper: Optimize 32-to-16-bpp 
>> conversion")
>> Fixes: 65931bbc5177 ("drm/format-helper: Optimize 32-to-8-bpp 
>> conversion")
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Thanks for the fix. I build with W=1 but never saw this error. Do you 
> use other build flags?

Just building with clang. No other special build flags.

> 
> Best regards
> Thomas
> 
>> ---
>>   drivers/gpu/drm/drm_format_helper.c | 32 ++++++++++++++++-------------
>>   1 file changed, 18 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/ 
>> drm_format_helper.c
>> index fc7347caf600..d36e6cacc575 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -261,10 +261,10 @@ static __always_inline void 
>> drm_fb_xfrm_line_32to8(void *dbuf, const void *sbuf,
>>       /* write 4 pixels at once */
>>       while (sbuf32 < ALIGN_DOWN_PIXELS(send32, pixels, 4)) {
>>           u32 pix[4] = {
>> -            le32_to_cpup(sbuf32++),
>> -            le32_to_cpup(sbuf32++),
>> -            le32_to_cpup(sbuf32++),
>> -            le32_to_cpup(sbuf32++),
>> +            le32_to_cpup(sbuf32),
>> +            le32_to_cpup(sbuf32 + 1),
>> +            le32_to_cpup(sbuf32 + 2),
>> +            le32_to_cpup(sbuf32 + 3),
>>           };
>>           /* write output bytes in reverse order for little endianness */
>>           u32 val32 = xfrm_pixel(pix[0]) |
>> @@ -272,6 +272,7 @@ static __always_inline void 
>> drm_fb_xfrm_line_32to8(void *dbuf, const void *sbuf,
>>                  (xfrm_pixel(pix[2]) << 16) |
>>                  (xfrm_pixel(pix[3]) << 24);
>>           *dbuf32++ = cpu_to_le32(val32);
>> +        sbuf32 += ARRAY_SIZE(pix);
>>       }
>>       /* write trailing pixels */
>> @@ -294,10 +295,10 @@ static __always_inline void 
>> drm_fb_xfrm_line_32to16(void *dbuf, const void *sbuf
>>       /* write 4 pixels at once */
>>       while (sbuf32 < ALIGN_DOWN_PIXELS(send32, pixels, 4)) {
>>           u32 pix[4] = {
>> -            le32_to_cpup(sbuf32++),
>> -            le32_to_cpup(sbuf32++),
>> -            le32_to_cpup(sbuf32++),
>> -            le32_to_cpup(sbuf32++),
>> +            le32_to_cpup(sbuf32),
>> +            le32_to_cpup(sbuf32 + 1),
>> +            le32_to_cpup(sbuf32 + 2),
>> +            le32_to_cpup(sbuf32 + 3),
>>           };
>>           /* write output bytes in reverse order for little endianness */
>>           u64 val64 = ((u64)xfrm_pixel(pix[0])) |
>> @@ -305,6 +306,7 @@ static __always_inline void 
>> drm_fb_xfrm_line_32to16(void *dbuf, const void *sbuf
>>                   ((u64)xfrm_pixel(pix[2]) << 32) |
>>                   ((u64)xfrm_pixel(pix[3]) << 48);
>>           *dbuf64++ = cpu_to_le64(val64);
>> +        sbuf32 += ARRAY_SIZE(pix);
>>       }
>>   #endif
>> @@ -312,13 +314,14 @@ static __always_inline void 
>> drm_fb_xfrm_line_32to16(void *dbuf, const void *sbuf
>>       dbuf32 = (__le32 __force *)dbuf64;
>>       while (sbuf32 < ALIGN_DOWN_PIXELS(send32, pixels, 2)) {
>>           u32 pix[2] = {
>> -            le32_to_cpup(sbuf32++),
>> -            le32_to_cpup(sbuf32++),
>> +            le32_to_cpup(sbuf32),
>> +            le32_to_cpup(sbuf32 + 1),
>>           };
>>           /* write output bytes in reverse order for little endianness */
>>           u32 val32 = xfrm_pixel(pix[0]) |
>>                  (xfrm_pixel(pix[1]) << 16);
>>           *dbuf32++ = cpu_to_le32(val32);
>> +        sbuf32 += ARRAY_SIZE(pix);
>>       }
>>       /* write trailing pixel */
>> @@ -339,10 +342,10 @@ static __always_inline void 
>> drm_fb_xfrm_line_32to24(void *dbuf, const void *sbuf
>>       /* write pixels in chunks of 4 */
>>       while (sbuf32 < ALIGN_DOWN_PIXELS(send32, pixels, 4)) {
>>           u32 val24[4] = {
>> -            xfrm_pixel(le32_to_cpup(sbuf32++)),
>> -            xfrm_pixel(le32_to_cpup(sbuf32++)),
>> -            xfrm_pixel(le32_to_cpup(sbuf32++)),
>> -            xfrm_pixel(le32_to_cpup(sbuf32++)),
>> +            xfrm_pixel(le32_to_cpup(sbuf32)),
>> +            xfrm_pixel(le32_to_cpup(sbuf32 + 1)),
>> +            xfrm_pixel(le32_to_cpup(sbuf32 + 2)),
>> +            xfrm_pixel(le32_to_cpup(sbuf32 + 3)),
>>           };
>>           u32 out32[3] = {
>>               /* write output bytes in reverse order for little 
>> endianness */
>> @@ -363,6 +366,7 @@ static __always_inline void 
>> drm_fb_xfrm_line_32to24(void *dbuf, const void *sbuf
>>           *dbuf32++ = cpu_to_le32(out32[0]);
>>           *dbuf32++ = cpu_to_le32(out32[1]);
>>           *dbuf32++ = cpu_to_le32(out32[2]);
>> +        sbuf32 += ARRAY_SIZE(val24);
>>       }
>>       /* write trailing pixel */
>