cmd/compile, cmd/asm: incorrect use of T0 (X5) register in riscv64 assembly causes RAS mismatch and performance issue

[BoyaoWang430]

Based on The RISC-V Instruction Set Manual, not only X0 register but also X5 register can be used to be a link register. As described in Chapter 2.5.1, when two different link registers (x1 and x5) are given as rs1 and rd, the Return-address prediction stacks(RAS) will pop then push.

The Go runtime’s handwritten assembly and the code generated by the Go compiler incorrectly emit JALR X1, X5 instruction. When a function call uses x5 as the source register for JALR, it causes unintended RAS behavior. Although functionally correct, these instructions make the RAS perform a pop-then-push instead of a normal push, breaking call/return pairing and significantly degrading front-end branch prediction performance.

[gabyhelp]

Related Issues

• riscv64: LR saved before SP update #38828 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[randall77]

@golang/riscv64

[mengzhuo]

Could you give a benchmark on "breaking call/return pairing and significantly degrading front-end branch prediction performance"?

IIRC, None of current hardware implementation follow this rule.

[wangpc-pp]

Could you give a benchmark on "breaking call/return pairing and significantly degrading front-end branch prediction performance"?

IIRC, None of current hardware implementation follow this rule.

I don't think so, almost all hardware implementations (even some RV32 cores) have RAS because this is the simplest way to reduce misprediction of call/return.

We have encouterred the same issue in OpenJDK but it seems to have been fixed: https://bugs.openjdk.org/browse/JDK-8340241

And in LLVM, both x1/x5 are excluded for JALR register allocation: https://github.com/llvm/llvm-project/blob/1f35b52a00ebd7d595deaffd5e72f72088f450b1/llvm/lib/Target/RISCV/RISCVRegisterInfo.td#L293-L297

We have fixed this in downstream Golang and we will post the CLs after validation.

[BoyaoWang430]

Could you give a benchmark on "breaking call/return pairing and significantly degrading front-end branch prediction performance"?

IIRC, None of current hardware implementation follow this rule.

pkg: golang.org/x/benchmarks
goos: linux
goarch: riscv64
       │ origin.txt  │              opt.txt               │
       │   sec/op    │   sec/op     vs base               │
JSON-8   47.84m ± 1%   44.17m ± 1%  -7.66% (p=0.000 n=20)

       │      origin.txt      │                   opt.txt                   │
       │ GC-bytes-from-system │ GC-bytes-from-system  vs base               │
JSON-8           3.465Mi ± 3%           3.557Mi ± 0%  +2.66% (p=0.045 n=20)

       │  origin.txt  │            opt.txt             │
       │  STW-sec/GC  │  STW-sec/GC   vs base          │
JSON-8   2.215m ± 16%   2.087m ± 16%  ~ (p=0.121 n=20)

       │  origin.txt  │               opt.txt               │
       │  STW-sec/op  │  STW-sec/op   vs base               │
JSON-8   205.4µ ± 13%   190.1µ ± 21%  -7.44% (p=0.040 n=20)

       │     origin.txt     │               opt.txt                │
       │ allocated-bytes/op │ allocated-bytes/op  vs base          │
JSON-8         4.979Mi ± 2%         4.949Mi ± 3%  ~ (p=0.114 n=20)

       │ origin.txt  │            opt.txt            │
       │  allocs/op  │  allocs/op   vs base          │
JSON-8   40.12k ± 0%   40.12k ± 0%  ~ (p=0.355 n=20)

       │    origin.txt     │                  opt.txt                  │
       │ bytes-from-system │ bytes-from-system  vs base                │
JSON-8        199.3Mi ± 6%        219.4Mi ± 0%  +10.12% (p=0.000 n=20)

       │       origin.txt       │                    opt.txt                     │
       │ heap-bytes-from-system │ heap-bytes-from-system  vs base                │
JSON-8             191.1Mi ± 6%             211.0Mi ± 0%  +10.41% (p=0.000 n=20)

       │       origin.txt        │                    opt.txt                     │
       │ other-bytes-from-system │ other-bytes-from-system  vs base               │
JSON-8              3.861Mi ± 0%              3.871Mi ± 0%  +0.25% (p=0.001 n=20)

       │   origin.txt   │                opt.txt                │
       │ peak-RSS-bytes │ peak-RSS-bytes  vs base               │
JSON-8     193.4Mi ± 1%     200.2Mi ± 0%  +3.47% (p=0.000 n=20)

       │  origin.txt   │               opt.txt                │
       │ peak-VM-bytes │ peak-VM-bytes  vs base               │
JSON-8    1.300Gi ± 5%    1.425Gi ± 0%  +9.57% (p=0.000 n=20)

       │       origin.txt        │                  opt.txt                  │
       │ stack-bytes-from-system │ stack-bytes-from-system  vs base          │
JSON-8              960.0Ki ± 7%            1024.0Ki ± 12%  ~ (p=0.429 n=20)

       │   origin.txt    │                opt.txt                 │
       │ user+sys-sec/op │ user+sys-sec/op  vs base               │
JSON-8       323.0m ± 0%       300.0m ± 0%  -7.11% (p=0.000 n=20)

I tested the json benchmark of go benchmark on SpacemiT K1(musebook). The fix version has a performace improvement of about 7%.

[gopherbot]

Change https://go.dev/cl/726760 mentions this issue: cmd/compile, runtime: avoid improper control transfer instruction hints on riscv64