| Message ID | 20250402104430.142398-2-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/format-helper: fix build | expand |
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? 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 */
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 */ >
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 */
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> --- drivers/gpu/drm/drm_format_helper.c | 32 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-)