Duplicate callbr destination!

[adelva1984]

Repro with:

$ mkdir kernel && cd kernel
$ repo init -u https://android.googlesource.com/kernel/manifest -b common-android12-5.4
$ repo sync -j32 -d
$ BUILD_CONFIG=common-modules/virtual-device/build.config.cuttlefish.i686 build/build.sh

Before building though, apply this hack:

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e3cb534b3e79..0a255ef1708c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -90,7 +90,7 @@ config X86
        select ARCH_SUPPORTS_ACPI
        select ARCH_SUPPORTS_ATOMIC_RMW
        select ARCH_SUPPORTS_NUMA_BALANCING     if X86_64
-       select ARCH_SUPPORTS_LTO_CLANG          if X86_64
+       select ARCH_SUPPORTS_LTO_CLANG
        select ARCH_SUPPORTS_THINLTO            if X86_64
        select ARCH_USE_BUILTIN_BSWAP
        select ARCH_USE_QUEUED_RWLOCKS

At link time, you will see:

Duplicate callbr destination!
  callbr void asm sideeffect "1: jmp 6f\0A2:\0A.skip -(((5f-4f) - (2b-1b)) > 0) * ((5f-4f) - (2b-1b)),0x90\0A3:\0A.section .altinstructions,\22a\22\0A .long 1b - .\0A .long 4f - .\0A .word ${1:P}\0A .byte 3b - 1b\0A .byte 5f - 4f\0A .byte 3b - 2b\0A.previous\0A.section .altinstr_replacement,\22ax\22\0A4: jmp ${5:l}\0A5:\0A.previous\0A.section .altinstructions,\22a\22\0A .long 1b - .\0A .long 0\0A .word ${0:P}\0A .byte 3b - 1b\0A .byte 0\0A .byte 0\0A.previous\0A.section .altinstr_aux,\22ax\22\0A6:\0A testb $2,$3\0A jnz ${4:l}\0A jmp ${5:l}\0A.previous\0A", "i,i,i,*m,X,X,~{dirflag},~{fpsr},~{flags}"(i16 515, i32 117, i32 8, i8* bitcast (i32* getelementptr inbounds (%struct.cpuinfo_x86, %struct.cpuinfo_x86* @boot_cpu_data, i32 0, i32 10, i32 16) to i8*), i8* blockaddress(@kvm_load_guest_xcr0, %22), i8* blockaddress(@kvm_load_guest_xcr0, %22)) #13
          to label %21 [label %22, label %22], !dbg !11371499, !srcloc !11370469
in function kvm_load_guest_xcr0
LLVM ERROR: Broken function found, compilation aborted!

[nickdesaulniers]

So, both GCC and Clang will prevent duplicate label operands for asm goto in C:

void foo(void) {
    asm goto (""::::bar, bar);
bar:;
}

produces an error in both compilers. What is permitted though is:

void foo(void) {
    asm goto (""::::bar, baz);
bar:
baz:;
}

For LLVM IR, it seems that we should be creating a new llvm::BasicBlock when we would otherwise be creating a duplicate parameter. We don't want to try to rewrite the inline asm (which we treat as a black box) to refer to the one shared parameter.

cc @gwelymernans @jyknight @topperc

[nickdesaulniers]

Interestingly, I'm not able to trigger this with upstream LLVM:

diff --git a/build.config.common b/build.config.common
index c73f0e74cfa0..52a9984aa8a3 100644
--- a/build.config.common
+++ b/build.config.common
@@ -4,6 +4,7 @@ KMI_GENERATION=0
 LLVM=1
 DEPMOD=depmod
 CLANG_PREBUILT_BIN=prebuilts-master/clang/host/linux-x86/clang-r383902/bin
+CLANG_PREBUILT_BIN=/android0/llvm-project/llvm/build/bin/clang
 BUILDTOOLS_PREBUILT_BIN=build/build-tools/path/linux-x86
 
 EXTRA_CMDS=''

Instead, I hit:

  LTO [M] sound/pci/snd-intel8x0.lto.o
Unsupported relocation type: R_386_PLT32 (4)
make[3]: *** [/android0/kernel-common2/common/arch/x86/boot/compressed/Makefile:135: arch/x86/boot/compressed/vmlinux.relocs] Error 1

So we succeed in linking vmlinux with LTO, but fail for a kernel module. That sounds like we might be closer to linking vmlinux 32b x86 with a newer compiler. cc @samitolvanen That looks like #579 aosp/1437431. Though I do have that applied, so I'm not immediately sure why we'd still see that error.

I should still probably relax the constraint about duplicate destinations. It's not invalid IR, just stupid/inefficient. It kind of bakes in constraints from the source language which I don't like.

[nathanchance]

I cannot reproduce this with the following diff on top of Linux 5.12 with i386_defconfig + CONFIG_LTO_CLANG_FULL=y and ToT LLVM.

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2792879d398e..b653842e7d59 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -97,7 +97,7 @@ config X86
        select ARCH_SUPPORTS_DEBUG_PAGEALLOC
        select ARCH_SUPPORTS_NUMA_BALANCING     if X86_64
        select ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP       if NR_CPUS <= 4096
-       select ARCH_SUPPORTS_LTO_CLANG          if X86_64
+       select ARCH_SUPPORTS_LTO_CLANG
        select ARCH_SUPPORTS_LTO_CLANG_THIN     if X86_64
        select ARCH_USE_BUILTIN_BSWAP
        select ARCH_USE_QUEUED_RWLOCKS

Closing for now.

[nickdesaulniers]

@nathanchance want to send that diff upstream?

[nathanchance]

Yes, further testing then sending that patch is on my TODO.