[RISC-V] Cannot relax `auipc` + `jr` after Machine Outliner

[kvpanch]

Consider following LLVM IR

target datalayout = "e-m:e-p:32:64-p1:32:64-i64:64-i128:128-n32:64-S64"
target triple = "riscv64-unknown--elf"

; Function Attrs: minsize
define i256 @func0() #0 {
entry:
  ret i256 0
}

; Function Attrs: minsize
define i256 @func1() #0 {
entry:
  ret i256 0
}

attributes #0 = { minsize }

Compiling it with llc -O3 test.ll -o test.s makes Machine Outliner to construct a function OUTLINED_FUNCTION_0 and add tail call to it to func0 and func1. This generates ELF file without

0000000000000000 <func0>:
       0: 00000317     	auipc	t1, 0x0
       4: 01030067     	jr	0x10(t1) <OUTLINED_FUNCTION_0>

0000000000000008 <func1>:
       8: 00000317     	auipc	t1, 0x0
       c: 00830067     	jr	0x8(t1) <OUTLINED_FUNCTION_0>

which cannot be relaxed as lld expects R_RISCV_CALL_PLT to be set.
For me it looks like something is not correct here.
One option to fix this is to use linkodronce linkage type for outlined function as comments states

[kvpanch]

cc @topperc . Not sure if you're the right person to answer about relaxation after machine outliner. If not, can you please add someone how can ? Thanks!

[topperc]

CC @MaskRay @lenary @jrtc27

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: None (kvpanch)

Consider following LLVM IR ``` target datalayout = "e-m:e-p:32:64-p1:32:64-i64:64-i128:128-n32:64-S64" target triple = "riscv64-unknown--elf"

; Function Attrs: minsize
define i256 @func0() #0 {
entry:
ret i256 0
}

; Function Attrs: minsize
define i256 @func1() #0 {
entry:
ret i256 0
}

attributes #0 = { minsize }

Compiling it with `llc -O3 test.ll -o test.s` makes Machine Outliner to construct a function `OUTLINED_FUNCTION_0` and add tail call to it to `func0` and `func1`. This generates ELF file without

0000000000000000 <func0>:
0: 00000317 auipc t1, 0x0
4: 01030067 jr 0x10(t1) <OUTLINED_FUNCTION_0>

0000000000000008 <func1>:
8: 00000317 auipc t1, 0x0
c: 00830067 jr 0x8(t1) <OUTLINED_FUNCTION_0>

which cannot be relaxed as `lld` expects `R_RISCV_CALL_PLT` to be set.
For me it looks like something is not correct here.
One option to fix this is to user `linkodronce` linkage type for outlined function as [comments states](https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/MachineOutliner.cpp#L922-L924)
</details>

[topperc]

@kvpanch do you know if it gets relaxed with GNU ld?

[lenary]

Relaxations are disabled by default in the backend, and you need --mattr=+relax to enable them.

$ llc -O3 main.ll -o main.o -filetype=obj -mattr=+relax
$ llvm-objdump -dr main.o                               

main.o: file format elf64-littleriscv

Disassembly of section .text:

0000000000000000 <func0>:
       0: 00000317      auipc   t1, 0x0
                0000000000000000:  R_RISCV_CALL_PLT     OUTLINED_FUNCTION_0
                0000000000000000:  R_RISCV_RELAX        *ABS*
       4: 00030067      jr      t1 <func0>
       8: 0001          <unknown>
                0000000000000008:  R_RISCV_ALIGN        *ABS*+0x2

000000000000000a <func1>:
       a: 00000317      auipc   t1, 0x0
                000000000000000a:  R_RISCV_CALL_PLT     OUTLINED_FUNCTION_0
                000000000000000a:  R_RISCV_RELAX        *ABS*
       e: 00030067      jr      t1 <func1>
      12: 0001          <unknown>
                0000000000000012:  R_RISCV_ALIGN        *ABS*+0x2

0000000000000014 <OUTLINED_FUNCTION_0>:
      14: 00053023      sd      zero, 0x0(a0)
      18: 00053423      sd      zero, 0x8(a0)
      1c: 00053823      sd      zero, 0x10(a0)
      20: 00053c23      sd      zero, 0x18(a0)
      24: 00008067      ret

Without --mattr=+relax, the assembler can evaluate the R_RISCV_CALL_PLT relocation at assembly time, because it is calling a function it knows is local, which is why those aren't seen when relaxation is disabled.

[lenary]

The difference between clang having relaxations enabled by default, and llc needing an additional target feature is not obvious at all, but I think it is the right thing to (continue to) do.

[lenary]

I'm not sure linkonce_odr is an allowable linkage for outlined functions. This says that functions with the same name are trusted to do the same thing. Outlined functions from the machine outliner have counter-based names, rather than their name being related to the outlined sequence, so I don't think we can deduplicate them based on their name.

[kvpanch]

Without --mattr=+relax, the assembler can evaluate the R_RISCV_CALL_PLT relocation at assembly time, because it is calling a function it knows is local, which is why those aren't seen when relaxation is disabled.

thank you. That's important extension I've missed.

The difference between clang having relaxations enabled by default, and llc needing an additional target feature is not obvious at all, but I think it is the right thing to (continue to) do.

Totally agree. But what's a reason to keep different default values in clang and in llc ?

[lenary]

The difference between clang having relaxations enabled by default, and llc needing an additional target feature is not obvious at all, but I think it is the right thing to (continue to) do.

Totally agree. But what's a reason to keep different default values in clang and in llc ?

IIRC, the original reason was that other parts of the toolchain (lld) didn't support linking objects made with relaxation enabled (because of R_RISCV_ALIGN), so we wanted to make it opt in, and then later we flipped clang's default to match GCC's default.

I think the reason we've stuck to this is that using target-features to only ever enable things are a lot more obvious to reason about in the backend than having some target features enable things and some disable things. This doesn't seem to be entirely followed any more, now we have quite a few disable-misched-* features, and no-trailing-seq-cst-fence.

But for relax specifically, I think there's still a good reason to leave it opt-in in the backend - if you're bringing up a new risc-v based tool of some kind, based on LLVM's codegen, you may not know about linker relaxations and so opting-into this complexity is nicer on newer projects rather than presenting it from the start.

[lenary]

Just going to mark this as "not planned" because no action was required.