On 7/16/2023 6:23 AM, Rémi Denis-Courmont wrote:
Le sunnuntaina 16. heinäkuuta 2023, 2.58.32 EEST James Almer a écrit :
Should fix assembling with binutil as >= 2.41

Signed-off-by: James Almer <jamr...@gmail.com>
---
This is IMO a big breakage. binutil's as has until now clipped these values
on its own, and never required the compiler to do it.

TBH, silently clipping immediate constants sounds like a nasty bug that could
cause really nasty suprises if somebody every passes an out-of-range constant.

We're passing it out or range constants alright. I tried adding an av_assert0((uint8_t)(-s) <= 31) and most fate tests started failing.

This has happened to me many times, typically with incidentally out-of-range
immediate offsets in loads/stores.

(...)

      __asm__ ("shrl %1, %0\n\t"
           : "+r" (a)
-         : "ic" ((uint8_t)(-s))
+         : "Ic" ((uint8_t)(-s))

Note that this is not equivalent. Now, if `s` is constant but out of range,
the compiler will be required to fit it. And it does that by moving it into
ECX. This is probably not what you want.

AFAICT, you should keep the constraint as it is, and fix the operand value
instead by masking it, e.g.:

         if (__builtin_constant_p(s))
                 __asm__ ("shrl %1, %0\n\t"
                         : "+r" (a)
                         : "i" ((-s) & 0x1f)
                 );
         else
                 __asm__ ("shrl %1, %0\n\t"
                         : "+r" (a)
                         : "c" (-s)
                 );

(Not sure if the the 0x1f mask is correct, but you get the idea.)

It is, just tested.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to