On Sun, 2 Jul 2023, Martin Storsjö wrote:
On Sun, 2 Jul 2023, John Cox wrote:
On Sun, 2 Jul 2023 00:35:14 +0300 (EEST), you wrote:
On Thu, 29 Jun 2023, John Cox wrote:
Add macros for dual scalar half->single multiply and accumulate
Add macro for shift, saturate and shorten single to byte
Add filter constants
Signed-off-by: John Cox <j...@kynesim.co.uk>
---
libavfilter/aarch64/vf_bwdif_neon.S | 46 +++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
+
+ .align 16
Note that .align for arm is power of two; this triggers a 2^16 byte
alignment here, which certainly isn't intended.
Yikes! I'll swap that for a .balign now I've looked that up
But in general, the usual way of defining constants is with a
const/endconst block, which places them in the right rdata section instead
of in the text section. But that then requires you to use a movrel macro
for accessing the data, instead of a plain ldr instruction.
Yeah - arm has a history of mixing text & const - I went with the
simpler code. Is this a deal breaker or can I leave it as is?
I wouldn't treat it as a deal breaker as long as it works across all
platforms - even if consistency with the code style elsewhere is preferred,
but IIRC there may be issues with MS armasm (after passed through
gas-preprocessor). IIRC there might be issues with starting out with straight
up content without the full setup made by the function/const macros. OTOH I
might have fixed that in gas-preprocessor too...
Last time around, the patchset failed building in that configuration due ot
the out of range alignment, I'll see how it fares now.
I'm sorry, but I'd just recommend you to go with the const macros.
Your current patch fails because gas-preprocessor,
https://github.com/ffmpeg/gas-preprocessor, doesn't support the .balign
directive, it only recognizes .align and .p2align. (Extending it to
support it would be trivial though.)
If I change your code to ".align 4", I get the following warning:
\home\martin\code\ffmpeg-msvc-arm64\libavfilter\aarch64\vf_bwdif_neon.o.asm(1011)
: warning A4228: Alignment value exceeds AREA alignment; alignment not
guaranteed
ALIGN 16
Since we haven't started any section, apparently armasm defaults to a
section with 4 byte alignment.
But anyway, regardless of the alignment, it later fails with this error:
\home\martin\code\ffmpeg-msvc-arm64\libavfilter\aarch64\vf_bwdif_neon.o.asm(1051)
: error A2504: operand 2: Expected address
ldr q0, coeffs
So I would request you to just go with the macros we use elsewhere. The
gas-preprocessor/armasm setup doesn't support/expect any random assembly,
but the disciplined subset we normally write. In most cases, we
essentially never write bare directives in the code, but only use the
macros from asm.S, which are set up to handle portability across all
supported platforms and their toolchains.
// Martin
_______________________________________________
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".
