ABI checks: add support for loongarch

[heiher]

LoongArch psABI¹ specifies that LSX vector types are passed via general-purpose registers, while LASX vector types are passed indirectly through the stack.

This patch addresses the following warnings:

warning: this function call uses a SIMD vector type that is not currently supported with the chosen ABI
    --> .../library/core/src/../../stdarch/crates/core_arch/src/loongarch64/lsx/generated.rs:3695:5
     |
3695 |     __lsx_vreplgr2vr_b(a)
     |     ^^^^^^^^^^^^^^^^^^^^^ function called here
     |
     = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
     = note: for more information, see issue #116558 <https://github.com/rust-lang/rust/issues/116558>
     = note: `#[warn(abi_unsupported_vector_types)]` on by default

r? @workingjubilee

Footnotes

1. https://github.com/loongson/la-abi-specs/blob/release/lapcs.adoc ↩

[workingjubilee]

lsx? lasx? oh dear.

[heiher]

lsx? lasx? oh dear.

LSX is a 128-bit vector extension, and LASX is a 256-bit vector extension. Parameters and return values for neither of these data types are passed through vector registers. 😃

[workingjubilee]

Then why is the lint triggering?

[workingjubilee]

Here is the uses_vector_registers check.

It looks fairly dubious to me. https://github.com/rust-lang/rust/blame/bfe809d93c67de03e3a84b80549927fd828ec5d0/compiler/rustc_monomorphize/src/mono_checks/abi_check.rs#L43

[workingjubilee]

This is not the correct fix, the correct fix is to fix the lint itself.

[heiher]

Here is the uses_vector_registers check.

It looks fairly dubious to me. https://github.com/rust-lang/rust/blame/bfe809d93c67de03e3a84b80549927fd828ec5d0/compiler/rustc_monomorphize/src/mono_checks/abi_check.rs#L43

I see it. But I'm not sure how to make it more accurate.

Here's a case:

#![feature(stdarch_loongarch)]
use std::arch::loongarch64::*;

pub unsafe fn simd(s: i32) -> u32 {
    lsx_vpickve2gr_bu::<0>(lsx_vreplgr2vr_b(s))
}

and the PassMode is

Direct(ArgAttributes { regular: , arg_ext: None, pointee_size: Size(0 bytes), pointee_align: None })

BackendRepr is

Vector { element: Initialized { value: Int(I8, true), valid_range: 0..=255 }, count: 16 }

[heiher]

Is it true that the Rust frontend cannot determine whether a parameter is passed through a vector register or a general-purpose register based solely on PassMode and BackendRepr? @veluca93 This decision is made by the LLVM target backend. If so, adding ABI checks to LoongArch is also a valid solution, and at least harmless.

[workingjubilee]

My understanding is the frontend technically cannot precisely control it, no.

[heiher]

Since we cannot determine the type of registers used for vectors in Rust's front-end, I have reverted the return type of features_for_correct_vector_abi to Option. A return value of None indicates that the given target does not use vector registers for vectors.

Do you think this is ok? @workingjubilee

[workingjubilee]

Hmm.

[workingjubilee]

What we want is to both

• understand what the C ABI uses
• understand what is possible at the level of codegen and the machine ISA, because we also need to do codegen correctly for intrinsics and other, non-C ABIs

If the backend does not expose some ability to handle register usage (which does imply that we should have a nonzero ability to get it wrong), then I think we will simply fuck things up in a different way from our current approaches.

[workingjubilee]

In either version of this PR that I have seen so far, it adjusts things in a way that fixes things for the C ABI in certain conditions. However, the motivation¹ for the changes are that our current compiler contract with the Loongarch backend remains poorly-specified and poorly-understood.

• The first version proposed, accordingly, seemed to just assuming that our compiler contract is similar to the contract with the other backends, when it apparently is not?
• The second version proposes surrendering to the whims of the codegen backend, in a way which may be technically correct but leaves us in the dark as we head into the future.

So, before I merge any fixes, I would like to understand, relative to how the Loongarch backend handles the LLVMIR lowering,

• what we can do
• what the effects of those choices are
• what we should be doing

Footnotes

1. I mean, the motivation strictly speaking is that we would like to not lint unnecessarily, but it is revealing that we do not understand exactly what is necessary. ↩

[heiher]

Okay. the following examples and LLVM IR shows how vector types are passed in the C ABI and Rust non-C ABI.

LSX (128-bit vectors)

• In the C ABI, 128-bit vector types are represented in LLVM IR as the i128 data type, which is split into two i64 data types and passed via general-purpose registers. (LLVM Backend: https://github.com/llvm/llvm-project/blob/53326ee0cf45fce3f80e2e98638dd27edb20c516/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp#L4811-L4961)

• In non-C ABI, 128-bit vector type data is allocated on the stack and passed by reference.

#![feature(stdarch_loongarch)]
#![feature(loongarch_target_feature)]
use std::arch::loongarch64::*;
use std::mem::transmute;

extern "C" fn no_target_feature(_dummy: f32, x: v4i32) {
    let val = unsafe { transmute::<_, [u32; 4]>(x) };
    dbg!(val);
}

#[target_feature(enable = "lsx")]
unsafe fn with_target_feature(x: v4i32) {
  no_target_feature(0.0, x);
}

fn main() {
    unsafe {
        with_target_feature(transmute([1; 4]));
    }
}

LLVM IR:

; lsx::no_target_feature
; Function Attrs: nounwind uwtable
define internal void @_ZN3lsx17no_target_feature17h2cf7e0ecaa1b600aE(float %0, i128 %1) unnamed_addr #3 personality ptr @rust_eh_personality {
  ;                                                                            ^--- by-val (i64, i64)
  ; ...
}

; lsx::with_target_feature
; Function Attrs: uwtable
define internal void @_ZN3lsx19with_target_feature17hfda3b7c2fa82d5f7E(ptr align 16 %x) unnamed_addr #4 {
  ;                                                                    ^--- by-ref
  ; ...
}

LASX (256-bit vectors)

• In both the C ABI and non-C ABI, 256-bit vector types are allocated on the stack and passed by reference.

#![feature(stdarch_loongarch)]
#![feature(loongarch_target_feature)]
use std::arch::loongarch64::*;
use std::mem::transmute;

extern "C" fn no_target_feature(_dummy: f32, x: v8i32) {
    let val = unsafe { transmute::<_, [u32; 8]>(x) };
    dbg!(val);
}

#[target_feature(enable = "lasx")]
unsafe fn with_target_feature(x: v8i32) {
  no_target_feature(0.0, x);
}

fn main() {
    unsafe {
        with_target_feature(transmute([1; 8]));
    }
}

; lasx::no_target_feature
; Function Attrs: nounwind uwtable
define internal void @_ZN4lasx17no_target_feature17h94e0853bc1ae88d5E(float %0, ptr align 32 %x) unnamed_addr #3 personality ptr @rust_eh_personality {
  ;                                                                             ^--- by-ref
  ; ...
}

; lasx::with_target_feature
; Function Attrs: uwtable
define internal void @_ZN4lasx19with_target_feature17h6ea2f98ed3995f8fE(ptr align 32 %x) unnamed_addr #4 {
  ;                                                                     ^--- by-ref
  ; ...
}

Vector registers are not used for parameter passing, and to maintain ABI compatibility, the calling convention will not be changed in the foreseeable future. That's why I marked LoongArch as not requiring vector ABI checks.

[heiher]

On some targets, the parameter passing conventions are controlled by the target features. If the caller and callee have different target features and the data type being passed is dependent on these features, inconsistencies can arise. The purpose of this ABI check is to detect such issues at compile-time. Technically, the parameter passing rules for vector data types in LoongArch are not affected by the target features, which applies to both C ABI and non-C ABI.

For the first version, due to the limitation that the Rust frontend cannot determine the register type used for passing vector types, we made a concession and accepted this unnecessary ABI check. It does not cause any issues.

For the second version, more technical analysis is required compared to the first. It is only feasible to proceed once it is confirmed that the parameter passing rules for LoongArch are not related to the target features. I would like to clarify that this is indeed the case for LoongArch.

Is there anything else we can do?

[heiher]

@workingjubilee (a gentle ping)

[heiher]

I attempted once again to find a direct approach to make uses_vector_registers more accurate in determining whether vector registers are used. Unfortunately, no such method was identified.

Regarding the indirect workaround, the first version appears to be preferable to the second. The second version bypasses the ABI check entirely for LoongArch, which means that if new vector data types are introduced in the future and indeed use vector registers, the ABI check would remain ineffective. In contrast, the first version aligns with the current design pattern. While the check might be redundant for the existing vector features, LSX and LASX, it provides greater flexibility for future extensibility.

[workingjubilee]

I see.

I think I agree the first version seems preferable, now, however this has drawn my attention:

LSX (128-bit vectors)

In non-C ABI, 128-bit vector type data is allocated on the stack and passed by reference.

LASX (256-bit vectors)
In both the C ABI and non-C ABI, 256-bit vector types are allocated on the stack and passed by reference.

Can we use the ptr byval representation in the Rust ABI? This is referred to locally as pass_by_stack_offset.

[heiher]

Can we use the ptr byval representation in the Rust ABI? This is referred to locally as pass_by_stack_offset.

Thanks for your reply. I’ll give it a try and propose it in a new PR. However, would switching to ptr byval affect Rust ABI compatibility?

[workingjubilee]

@heiher There is no such thing as ABI compatibility between versions of the compiler for Rust. The Rust ABI code is only guaranteed compatibility for a single version of the compiler.

[workingjubilee]

And yes, handling the actual ABI change in a separate PR is fine.

@bors r+ rollup

[bors]

📌 Commit 78f3946 has been approved by workingjubilee

It is now in the queue for this repository.