[FFmpeg-devel] [PATCH 1/4] configure: aarch64: Support assembling the dotprod and i8mm arch extensions

[Rémi Denis-Courmont]

Le sunnuntaina 28. toukokuuta 2023, 0.34.15 EEST Martin Storsjö a écrit :
> > Someone would have to write assembler code that would fail to build under
> > a toolchain with a lower target version. That sounds like a bug that
> > should be spotted and fixed, rather than papered over.
> 
> I don't see how anything here suggests papering over such an issue?

For instance, say the toolchain, or the FFmpeg build flags, sets ARMv8.6 as 
target architecture. It makes perfect sense for the C compiler to emit non-
hint ARMv8.3 PAuth instructions, which would crash on an ARMv8.0-8.2 
processor. But it makes zero sense for the assembler to change its behaviour. 
So all existing ASIMD assembler files written for ARMv8.0-A, should still 
generate ARMv8.0-A code.

But now somebody can accidentally write an ARMv8.1-8.6 instruction into their 
ARMv8.0 assembler code, and it will not trigger a build error (to them). There 
are no clear reasons to trying to avoid *lowering* the target version for 
*assembler* (as opposed to C).

If a file needs ARMv8.2, it should just ARMv8.2 as its target, whether the 
overall build targets ARMv8.0, ARMv8.2 or ARMv8.6. The C code around it should 
ensure that the requirements are met. There are no benefits to preserving a 
higher version; it just makes the configure checks needlessly intricate.

> if we do add ".arch armv8.2-a" and ".arch_extension i8mm", then we
> break the dotprod extension. If we only add ".arch_extension i8mm" without
> the .arch directive, we get what we want to though.

Yes, but you can also just set `.arch armv8.4-a`, which is ostensibly The Way 
to enable DotProd on a craptastic assembler. If even that doesn't work, the 
assembler presumably doesn't support DotProd at all (or it's some obscure 
unstable development version that was snapshot between adding DotProd and 
adding complete ARMv8.4, which we really should not care about).

Likewise, for I8MM, you can just do `.arch armv8.6-a`.

So what if the target version was 8.7 or 9.2? Well, in an assembler file, we 
don't really care, as noted above.

> > If the problem is to avoid `.arch_extension`, then I don't really see
> > why you can't just use `.arch` with plus, and simplify a lot.
> 
> Well Clang doesn't quite support that currently either. For
> ".arch_extension dotprod" it errors out since it doesn't recognize the
> dotprod feature in that directive. It does accept ".arch
> armv8.2-a+dotprod" but it doesn't actually unlock using the dotprod
> extension in the assembly despite that. (I'll look into fixing this in
> upstream LLVM afterwards.)

I don't know if `.arch_extension` is specified by Arm or just a GCCism. But if 
you accept DotProd in `.arch` in arch and then don't enable it, then that's 
clearly a bug. But then again, that's moot if you can just do `.arch armv8.4-
a` instead.

> Taking it back to the drawing board: So for enabling e.g. i8mm, we could
> either do
>      .arch armv8.6-a
> or
>      .arch armv8.2-a+dotprod
> or
>      .arch armv8.2
>      .arch_extension dotprod
> 
> 
> Out of these, I initially preferred doing the third approach.

I agree that that's the cleanest option. But that's not the point here. The 
point is that what this patch is a hell of a lot more involved than doing just 
that, and it seems unnecessarily intricate for the purpose of enabling 
DotProd.

> There's no functional difference between the second and third one, except
> the single-line form is more messy to handle, as we can have various
> combinations of what actually is supported.

Yes but a pile of #if's is even more messy to handle.

> And with the single-line .arch
> form, we can't just add e.g. i8mm on top of a -march= setting that already
> supports dotprod, without respecifying what the toolchain itself defaults
> to.
> 
> 
> The documentation for .arch_extension hints at it being possible to
> disable support for extensions with it too, but that doesn't seem to be
> the case in practice. If it was, we could add macros to only enable
> specifically the extensions we want around those functions that should use
> them and nothing more. But I guess if that's not actually supported we
> can't do that.

GCC supports it with RV64 (`.option arch` plus/minus). I haven't tried it on 
AArch64. Of course, LLVM does not support it, even though the patch has been 
out there for a long time and even though it is part of the ABI spec rather 
than made up by GNU/binutils. Thanks to that, RVV is unusable on LLVM (Linux 
kernel specifically requires GNU/as due to that).

> I guess the alternative would be to just try to set .arch
> <highest-supported-that-we-care-about>. I was worried that support for
> e.g. armv8.6-a appeared later in toolchains than support for the
> individual extension i8mm, but at least from a quick browse in binutils
> history, they seem to have been added at the same time, so there's
> probably no such drawback.
> 
> Or what's the situation with e.g. SVE2 - was ".arch_extension sve2"
> supported significantly earlier than ".arch armv9-a"?

I have not tested SVE on LLVM. AFAIK, SVE and SVE2 are optional from 8.2 and 
9.0 onward respectively, and not mandatory in any version, so if your 
toolchain supports neither .arch with plus sign, nor .arch_extension, it is 
game over.

> If we'd do that, it does simplify the configure logic a fair bit and
> reduces the number of configure variables we need by a lot. It does enable
> a few more instruction set extensions than what we need though, but that's
> probably not a real issue.

Yes.

-- 
Реми Дёни-Курмон
http://www.remlab.net/