Re: [PATCH v2] Make iwmmxt.S support Clang's integrated assembler

[Ard Biesheuvel]

On Tue, 10 Nov 2020 at 02:55, Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote:
>
> On Sat, Nov 7, 2020 at 12:29 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >
> > On Sat, 7 Nov 2020 at 01:11, Jian Cai <jiancai@xxxxxxxxxx> wrote:
> > >
> > > This patch replaces 6 IWMMXT instructions Clang's integrated assembler
> > > does not support in iwmmxt.S using macros, while making sure GNU
> > > assembler still emit the same instructions. This should be easier than
> > > providing full IWMMXT support in Clang.
> > >
> > > "Intel Wireless MMX Technology - Developer Guide - August, 2002" should
> > > be referenced for the encoding schemes of these extensions.
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/975
> > >
> > > Suggested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > > Suggested-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > Signed-off-by: Jian Cai <jiancai@xxxxxxxxxx>
> >
> > Please make sure you test this carefully on BE32, as the instruction
> > byte order used by .inst is LE IIRC
>
> Unless that was a recent-ish change in GAS, it looks like I get the
> same disassembly/encodings before/after this patch for
> CONFIG_BIG_ENDIAN with GNU as.
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make CC=clang -j71 defconfig
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make CC=clang -j71 menuconfig
> <enable CPU_BIG_ENDIAN>
> $ grep BIG_ENDIAN .config
> CONFIG_CPU_BIG_ENDIAN=y
> CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make CC=clang -j71
> arch/arm/kernel/iwmmxt.o
> $ arm-linux-gnueabi-objdump -dr arch/arm/kernel/iwmmxt.o > gas_before.txt
> $ b4 am https://lore.kernel.org/lkml/20201107001056.225807-1-jiancai@xxxxxxxxxx/
> -o - | git am
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make CC=clang -j71
> arch/arm/kernel/iwmmxt.o
> $ arm-linux-gnueabi-objdump -dr arch/arm/kernel/iwmmxt.o > gas_after.txt
> $ diff -u gas_before.txt gas_after.txt
> $ echo $?
> 0
>
> (Orthogonal, it looks like llvm-objdump has issues decoding
> elf32-bigarm that we'll need to fix.)
>
> Either way the patch LGTM for IWMMXT, thanks Jian for the patch and
> Ard for the suggestions.
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> Tested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>


BE32 != BE8

Please use, e.g., ixp4xx_defconfig with IWMMXT and BE enabled. It
seems like .inst does the right thing here, i.e., the assembler knows
that it should emit BE for BE32 and LE for BE8, but it needs to be
confirmed.