Re: [FFmpeg-devel] [PATCH] avcodec/x86/mathops: use constrained immediate operands

["Rémi Denis-Courmont" <remi@remlab.net>]

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 <jamrial@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. 
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.)

-- 
Rémi Denis-Courmont
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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